[Qemu-devel] [PATCH] ehci: avoid string arguments in trace events

[Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>]

String arguments are not supported by all trace backends.  This patch
replaces existing string arguments in hw/usb-ehci.c either with
individual trace events that remain human-friendly or by printing raw
addresses when there is no alternative or downside to that.

States and usbsts bits remain human-friendly since it is hard to
remember all of them.  MMIO addresses are printed raw because they would
create many individual trace events and the addresses are usually easy
to remember when debugging.  Queue actions remain human-friendly while
device attach only prints the USBDevice pointer.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/usb-ehci.c |  138 +++++++++++++++++++++++++++-----------------------------
 trace-events  |   40 ++++++++++++++---
 2 files changed, 100 insertions(+), 78 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 47a7fb9..e101fc7 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -435,93 +435,89 @@ struct EHCIState {
     *data = val; \
     } while(0)
 
-static const char *ehci_state_names[] = {
-    [ EST_INACTIVE ]     = "INACTIVE",
-    [ EST_ACTIVE ]       = "ACTIVE",
-    [ EST_EXECUTING ]    = "EXECUTING",
-    [ EST_SLEEPING ]     = "SLEEPING",
-    [ EST_WAITLISTHEAD ] = "WAITLISTHEAD",
-    [ EST_FETCHENTRY ]   = "FETCH ENTRY",
-    [ EST_FETCHQH ]      = "FETCH QH",
-    [ EST_FETCHITD ]     = "FETCH ITD",
-    [ EST_ADVANCEQUEUE ] = "ADVANCEQUEUE",
-    [ EST_FETCHQTD ]     = "FETCH QTD",
-    [ EST_EXECUTE ]      = "EXECUTE",
-    [ EST_WRITEBACK ]    = "WRITEBACK",
-    [ EST_HORIZONTALQH ] = "HORIZONTALQH",
-};
-
-static const char *ehci_mmio_names[] = {
-    [ CAPLENGTH ]        = "CAPLENGTH",
-    [ HCIVERSION ]       = "HCIVERSION",
-    [ HCSPARAMS ]        = "HCSPARAMS",
-    [ HCCPARAMS ]        = "HCCPARAMS",
-    [ USBCMD ]           = "USBCMD",
-    [ USBSTS ]           = "USBSTS",
-    [ USBINTR ]          = "USBINTR",
-    [ FRINDEX ]          = "FRINDEX",
-    [ PERIODICLISTBASE ] = "P-LIST BASE",
-    [ ASYNCLISTADDR ]    = "A-LIST ADDR",
-    [ PORTSC_BEGIN ]     = "PORTSC #0",
-    [ PORTSC_BEGIN + 4]  = "PORTSC #1",
-    [ PORTSC_BEGIN + 8]  = "PORTSC #2",
-    [ PORTSC_BEGIN + 12] = "PORTSC #3",
-    [ CONFIGFLAG ]       = "CONFIGFLAG",
-};
-
-static const char *nr2str(const char **n, size_t len, uint32_t nr)
+/* Individual human-friendly trace events for states */
+static void ehci_trace_set_state(int async, uint32_t state)
 {
-    if (nr < len && n[nr] != NULL) {
-        return n[nr];
-    } else {
-        return "unknown";
+    switch (state) {
+    case EST_INACTIVE:
+        trace_usb_ehci_state_inactive(async);
+        break;
+    case EST_ACTIVE:
+        trace_usb_ehci_state_active(async);
+        break;
+    case EST_EXECUTING:
+        trace_usb_ehci_state_executing(async);
+        break;
+    case EST_SLEEPING:
+        trace_usb_ehci_state_sleeping(async);
+        break;
+    case EST_WAITLISTHEAD:
+        trace_usb_ehci_state_waitlisthead(async);
+        break;
+    case EST_FETCHENTRY:
+        trace_usb_ehci_state_fetchentry(async);
+        break;
+    case EST_FETCHQH:
+        trace_usb_ehci_state_fetchqh(async);
+        break;
+    case EST_FETCHITD:
+        trace_usb_ehci_state_fetchitd(async);
+        break;
+    case EST_ADVANCEQUEUE:
+        trace_usb_ehci_state_advancequeue(async);
+        break;
+    case EST_FETCHQTD:
+        trace_usb_ehci_state_fetchqtd(async);
+        break;
+    case EST_EXECUTE:
+        trace_usb_ehci_state_execute(async);
+        break;
+    case EST_WRITEBACK:
+        trace_usb_ehci_state_writeback(async);
+        break;
+    case EST_HORIZONTALQH:
+        trace_usb_ehci_state_horizontalqh(async);
+        break;
+    default:
+        trace_usb_ehci_state_unknown(async, state);
+        break;
     }
 }
 
-static const char *state2str(uint32_t state)
-{
-    return nr2str(ehci_state_names, ARRAY_SIZE(ehci_state_names), state);
-}
-
-static const char *addr2str(target_phys_addr_t addr)
-{
-    return nr2str(ehci_mmio_names, ARRAY_SIZE(ehci_mmio_names), addr);
-}
-
 static void ehci_trace_usbsts(uint32_t mask, int state)
 {
     /* interrupts */
     if (mask & USBSTS_INT) {
-        trace_usb_ehci_usbsts("INT", state);
+        trace_usb_ehci_usbsts_int(state);
     }
     if (mask & USBSTS_ERRINT) {
-        trace_usb_ehci_usbsts("ERRINT", state);
+        trace_usb_ehci_usbsts_errint(state);
     }
     if (mask & USBSTS_PCD) {
-        trace_usb_ehci_usbsts("PCD", state);
+        trace_usb_ehci_usbsts_pcd(state);
     }
     if (mask & USBSTS_FLR) {
-        trace_usb_ehci_usbsts("FLR", state);
+        trace_usb_ehci_usbsts_flr(state);
     }
     if (mask & USBSTS_HSE) {
-        trace_usb_ehci_usbsts("HSE", state);
+        trace_usb_ehci_usbsts_hse(state);
     }
     if (mask & USBSTS_IAA) {
-        trace_usb_ehci_usbsts("IAA", state);
+        trace_usb_ehci_usbsts_iaa(state);
     }
 
     /* status */
     if (mask & USBSTS_HALT) {
-        trace_usb_ehci_usbsts("HALT", state);
+        trace_usb_ehci_usbsts_halt(state);
     }
     if (mask & USBSTS_REC) {
-        trace_usb_ehci_usbsts("REC", state);
+        trace_usb_ehci_usbsts_rec(state);
     }
     if (mask & USBSTS_PSS) {
-        trace_usb_ehci_usbsts("PSS", state);
+        trace_usb_ehci_usbsts_pss(state);
     }
     if (mask & USBSTS_ASS) {
-        trace_usb_ehci_usbsts("ASS", state);
+        trace_usb_ehci_usbsts_ass(state);
     }
 }
 
@@ -574,11 +570,11 @@ static inline void ehci_commit_interrupt(EHCIState *s)
 
 static void ehci_set_state(EHCIState *s, int async, int state)
 {
+    ehci_trace_set_state(async, state);
+
     if (async) {
-        trace_usb_ehci_state("async", state2str(state));
         s->astate = state;
     } else {
-        trace_usb_ehci_state("periodic", state2str(state));
         s->pstate = state;
     }
 }
@@ -656,13 +652,13 @@ static EHCIQueue *ehci_alloc_queue(EHCIState *ehci, int async)
     q->ehci = ehci;
     q->async_schedule = async;
     QTAILQ_INSERT_HEAD(&ehci->queues, q, next);
-    trace_usb_ehci_queue_action(q, "alloc");
+    trace_usb_ehci_alloc_queue(q);
     return q;
 }
 
 static void ehci_free_queue(EHCIQueue *q)
 {
-    trace_usb_ehci_queue_action(q, "free");
+    trace_usb_ehci_free_queue(q);
     if (q->async == EHCI_ASYNC_INFLIGHT) {
         usb_cancel_packet(&q->packet);
     }
@@ -728,7 +724,7 @@ static void ehci_attach(USBPort *port)
     EHCIState *s = port->opaque;
     uint32_t *portsc = &s->portsc[port->index];
 
-    trace_usb_ehci_port_attach(port->index, port->dev->product_desc);
+    trace_usb_ehci_port_attach(port->index, port->dev);
 
     if (*portsc & PORTSC_POWNER) {
         USBPort *companion = s->companion_ports[port->index];
@@ -905,7 +901,7 @@ static uint32_t ehci_mem_readl(void *ptr, target_phys_addr_t addr)
     val = s->mmio[addr] | (s->mmio[addr+1] << 8) |
           (s->mmio[addr+2] << 16) | (s->mmio[addr+3] << 24);
 
-    trace_usb_ehci_mmio_readl(addr, addr2str(addr), val);
+    trace_usb_ehci_mmio_readl(addr, val);
     return val;
 }
 
@@ -995,7 +991,7 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
     uint32_t old = *mmio;
     int i;
 
-    trace_usb_ehci_mmio_writel(addr, addr2str(addr), val);
+    trace_usb_ehci_mmio_writel(addr, val);
 
     /* Only aligned reads are allowed on OHCI */
     if (addr & 3) {
@@ -1006,7 +1002,7 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
 
     if (addr >= PORTSC && addr < PORTSC + 4 * NB_PORTS) {
         handle_port_status_write(s, (addr-PORTSC)/4, val);
-        trace_usb_ehci_mmio_change(addr, addr2str(addr), *mmio, old);
+        trace_usb_ehci_mmio_change(addr, *mmio, old);
         return;
     }
 
@@ -1086,7 +1082,7 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
     }
 
     *mmio = val;
-    trace_usb_ehci_mmio_change(addr, addr2str(addr), *mmio, old);
+    trace_usb_ehci_mmio_change(addr, *mmio, old);
 }
 
 
@@ -1231,7 +1227,7 @@ static void ehci_async_complete_packet(USBPort *port, USBPacket *packet)
     }
 
     q = container_of(packet, EHCIQueue, packet);
-    trace_usb_ehci_queue_action(q, "wakeup");
+    trace_usb_ehci_wakeup_queue(q);
     assert(q->async == EHCI_ASYNC_INFLIGHT);
     q->async = EHCI_ASYNC_FINISHED;
     q->usb_status = packet->result;
@@ -1626,7 +1622,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
     }
     if (q->async == EHCI_ASYNC_FINISHED) {
         /* I/O finished -- continue processing queue */
-        trace_usb_ehci_queue_action(q, "resume");
+        trace_usb_ehci_resume_queue(q);
         ehci_set_state(ehci, async, EST_EXECUTING);
         goto out;
     }
@@ -1839,7 +1835,7 @@ static int ehci_state_execute(EHCIQueue *q, int async)
     }
     if (q->usb_status == USB_RET_ASYNC) {
         ehci_flush_qh(q);
-        trace_usb_ehci_queue_action(q, "suspend");
+        trace_usb_ehci_suspend_queue(q);
         q->async = EHCI_ASYNC_INFLIGHT;
         ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
         again = 1;
diff --git a/trace-events b/trace-events
index 08ffedf..120d731 100644
--- a/trace-events
+++ b/trace-events
@@ -211,11 +211,33 @@ sun4m_iommu_bad_addr(uint64_t addr) "bad addr %"PRIx64""
 
 # hw/usb-ehci.c
 usb_ehci_reset(void) "=== RESET ==="
-usb_ehci_mmio_readl(uint32_t addr, const char *str, uint32_t val) "rd mmio %04x [%s] = %x"
-usb_ehci_mmio_writel(uint32_t addr, const char *str, uint32_t val) "wr mmio %04x [%s] = %x"
-usb_ehci_mmio_change(uint32_t addr, const char *str, uint32_t new, uint32_t old) "ch mmio %04x [%s] = %x (old: %x)"
-usb_ehci_usbsts(const char *sts, int state) "usbsts %s %d"
-usb_ehci_state(const char *schedule, const char *state) "%s schedule %s"
+usb_ehci_mmio_readl(uint32_t addr, uint32_t val) "rd mmio %04x = %x"
+usb_ehci_mmio_writel(uint32_t addr, uint32_t val) "wr mmio %04x = %x"
+usb_ehci_mmio_change(uint32_t addr, uint32_t new, uint32_t old) "ch mmio %04x = %x (old: %x)"
+usb_ehci_usbsts_int(int state) "usbsts INT %d"
+usb_ehci_usbsts_errint(int state) "usbsts ERRINT %d"
+usb_ehci_usbsts_pcd(int state) "usbsts PCD %d"
+usb_ehci_usbsts_flr(int state) "usbsts FLR %d"
+usb_ehci_usbsts_hse(int state) "usbsts HSE %d"
+usb_ehci_usbsts_iaa(int state) "usbsts IAA %d"
+usb_ehci_usbsts_halt(int state) "usbsts HALT %d"
+usb_ehci_usbsts_rec(int state) "usbsts REC %d"
+usb_ehci_usbsts_pss(int state) "usbsts PSS %d"
+usb_ehci_usbsts_ass(int state) "usbsts ASS %d"
+usb_ehci_state_inactive(bool async) "schedule async %d INACTIVE"
+usb_ehci_state_active(bool async) "schedule async %d ACTIVE"
+usb_ehci_state_executing(bool async) "schedule async %d EXECUTING"
+usb_ehci_state_sleeping(bool async) "schedule async %d SLEEPING"
+usb_ehci_state_waitlisthead(bool async) "schedule async %d WAITLISTHEAD"
+usb_ehci_state_fetchentry(bool async) "schedule async %d FETCH ENTRY"
+usb_ehci_state_fetchqh(bool async) "schedule async %d FETCH QH"
+usb_ehci_state_fetchitd(bool async) "schedule async %d FETCH ITD"
+usb_ehci_state_advancequeue(bool async) "schedule async %d ADVANCE QUEUE"
+usb_ehci_state_fetchqtd(bool async) "schedule async %d FETCH QTD"
+usb_ehci_state_execute(bool async) "schedule async %d EXECUTE"
+usb_ehci_state_writeback(bool async) "schedule async %d WRITEBACK"
+usb_ehci_state_horizontalqh(bool async) "schedule async %d HORIZONTAL QH"
+usb_ehci_state_unknown(bool async, uint32_t state) "schedule async %d state %08x"
 usb_ehci_qh_ptrs(void *q, uint32_t addr, uint32_t nxt, uint32_t c_qtd, uint32_t n_qtd, uint32_t a_qtd) "q %p - QH @ %08x: next %08x qtds %08x,%08x,%08x"
 usb_ehci_qh_fields(uint32_t addr, int rl, int mplen, int eps, int ep, int devaddr) "QH @ %08x - rl %d, mplen %d, eps %d, ep %d, dev %d"
 usb_ehci_qh_bits(uint32_t addr, int c, int h, int dtc, int i) "QH @ %08x - c %d, h %d, dtc %d, i %d"
@@ -223,11 +245,15 @@ usb_ehci_qtd_ptrs(void *q, uint32_t addr, uint32_t nxt, uint32_t altnext) "q %p
 usb_ehci_qtd_fields(uint32_t addr, int tbytes, int cpage, int cerr, int pid) "QTD @ %08x - tbytes %d, cpage %d, cerr %d, pid %d"
 usb_ehci_qtd_bits(uint32_t addr, int ioc, int active, int halt, int babble, int xacterr) "QTD @ %08x - ioc %d, active %d, halt %d, babble %d, xacterr %d"
 usb_ehci_itd(uint32_t addr, uint32_t nxt, uint32_t mplen, uint32_t mult, uint32_t ep, uint32_t devaddr) "ITD @ %08x: next %08x - mplen %d, mult %d, ep %d, dev %d"
-usb_ehci_port_attach(uint32_t port, const char *device) "attach port #%d - %s"
+usb_ehci_port_attach(uint32_t port, void *dev) "attach port #%d - %p"
 usb_ehci_port_detach(uint32_t port) "detach port #%d"
 usb_ehci_port_reset(uint32_t port, int enable) "reset port #%d - %d"
 usb_ehci_data(int rw, uint32_t cpage, uint32_t offset, uint32_t addr, uint32_t len, uint32_t bufpos) "write %d, cpage %d, offset 0x%03x, addr 0x%08x, len %d, bufpos %d"
-usb_ehci_queue_action(void *q, const char *action) "q %p: %s"
+usb_ehci_alloc_queue(void *q) "q %p"
+usb_ehci_free_queue(void *q) "q %p"
+usb_ehci_wakeup_queue(void *q) "q %p"
+usb_ehci_resume_queue(void *q) "q %p"
+usb_ehci_suspend_queue(void *q) "q %p"
 
 # hw/usb-desc.c
 usb_desc_device(int addr, int len, int ret) "dev %d query device, len %d, ret %d"
-- 
1.7.5.4