Re: [PATCH v2] e1000: Convert debug macros into tracepoints.
On 4/3/24 2:44 PM, Austin Clements wrote: At this point there's not much of my original code left. :D Don, you're welcome to take the credit in the commit. Thanks Austin. I'll send v3 with this change :) BTW, my attempt to include the appropriate maintainer from scripts/get_maintainer.pl (jasonw...@redhat.com) bounced. Any pointers on who else should be cc-ed are appreciated. -dp
Re: [PATCH v2] e1000: Convert debug macros into tracepoints.
At this point there's not much of my original code left. :D Don, you're welcome to take the credit in the commit. On Wed, Apr 3, 2024, 9:46 AM Don Porter wrote: > From: Austin Clements > > The E1000 debug messages are very useful for developing drivers. > Make these available to users without recompiling QEMU. > > Signed-off-by: Austin Clements > [geo...@ldpreload.com: Rebased on top of 2.9.0] > Signed-off-by: Geoffrey Thomas > Signed-off-by: Don Porter > --- > hw/net/e1000.c | 90 +++-- > hw/net/trace-events | 25 - > 2 files changed, 54 insertions(+), 61 deletions(-) > > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index 43f3a4a701..24475636a3 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -44,26 +44,6 @@ > #include "trace.h" > #include "qom/object.h" > > -/* #define E1000_DEBUG */ > - > -#ifdef E1000_DEBUG > -enum { > -DEBUG_GENERAL, DEBUG_IO, DEBUG_MMIO, DEBUG_INTERRUPT, > -DEBUG_RX, DEBUG_TX, DEBUG_MDIC, DEBUG_EEPROM, > -DEBUG_UNKNOWN, DEBUG_TXSUM,DEBUG_TXERR,DEBUG_RXERR, > -DEBUG_RXFILTER, DEBUG_PHY, DEBUG_NOTYET, > -}; > -#define DBGBIT(x)(1< -static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL); > - > -#define DBGOUT(what, fmt, ...) do { \ > -if (debugflags & DBGBIT(what)) \ > -fprintf(stderr, "e1000: " fmt, ## __VA_ARGS__); \ > -} while (0) > -#else > -#define DBGOUT(what, fmt, ...) do {} while (0) > -#endif > - > #define IOPORT_SIZE 0x40 > #define PNPMMIO_SIZE 0x2 > > @@ -351,8 +331,7 @@ e1000_mit_timer(void *opaque) > static void > set_ics(E1000State *s, int index, uint32_t val) > { > -DBGOUT(INTERRUPT, "set_ics %x, ICR %x, IMR %x\n", val, > s->mac_reg[ICR], > -s->mac_reg[IMS]); > +trace_e1000_set_ics(val, s->mac_reg[ICR], s->mac_reg[IMS]); > set_interrupt_cause(s, 0, val | s->mac_reg[ICR]); > } > > @@ -425,8 +404,7 @@ set_rx_control(E1000State *s, int index, uint32_t val) > s->mac_reg[RCTL] = val; > s->rxbuf_size = e1000x_rxbufsize(val); > s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1; > -DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT], > - s->mac_reg[RCTL]); > +trace_e1000_set_rx_control(s->mac_reg[RDT], s->mac_reg[RCTL]); > timer_mod(s->flush_queue_timer, >qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000); > } > @@ -440,16 +418,16 @@ set_mdic(E1000State *s, int index, uint32_t val) > if ((val & E1000_MDIC_PHY_MASK) >> E1000_MDIC_PHY_SHIFT != 1) // phy # > val = s->mac_reg[MDIC] | E1000_MDIC_ERROR; > else if (val & E1000_MDIC_OP_READ) { > -DBGOUT(MDIC, "MDIC read reg 0x%x\n", addr); > +trace_e1000_mdic_read_register(addr); > if (!(phy_regcap[addr] & PHY_R)) { > -DBGOUT(MDIC, "MDIC read reg %x unhandled\n", addr); > +trace_e1000_mdic_read_register_unhandled(addr); > val |= E1000_MDIC_ERROR; > } else > val = (val ^ data) | s->phy_reg[addr]; > } else if (val & E1000_MDIC_OP_WRITE) { > -DBGOUT(MDIC, "MDIC write reg 0x%x, value 0x%x\n", addr, data); > +trace_e1000_mdic_write_register(addr, data); > if (!(phy_regcap[addr] & PHY_W)) { > -DBGOUT(MDIC, "MDIC write reg %x unhandled\n", addr); > +trace_e1000_mdic_write_register_unhandled(addr); > val |= E1000_MDIC_ERROR; > } else { > if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) { > @@ -471,8 +449,8 @@ get_eecd(E1000State *s, int index) > { > uint32_t ret = E1000_EECD_PRES|E1000_EECD_GNT | > s->eecd_state.old_eecd; > > -DBGOUT(EEPROM, "reading eeprom bit %d (reading %d)\n", > - s->eecd_state.bitnum_out, s->eecd_state.reading); > +trace_e1000_get_eecd(s->eecd_state.bitnum_out, s->eecd_state.reading); > + > if (!s->eecd_state.reading || > ((s->eeprom_data[(s->eecd_state.bitnum_out >> 4) & 0x3f] >> >((s->eecd_state.bitnum_out & 0xf) ^ 0xf))) & 1) > @@ -511,9 +489,8 @@ set_eecd(E1000State *s, int index, uint32_t val) > s->eecd_state.reading = (((s->eecd_state.val_in >> 6) & 7) == > EEPROM_READ_OPCODE_MICROWIRE); > } > -DBGOUT(EEPROM, "eeprom bitnum in %d out %d, reading %d\n", > - s->eecd_state.bitnum_in, s->eecd_state.bitnum_out, > - s->eecd_state.reading); > +trace_e1000_set_eecd(s->eecd_state.bitnum_in, > s->eecd_state.bitnum_out, > + s->eecd_state.reading); > } > > static uint32_t > @@ -580,8 +557,7 @@ xmit_seg(E1000State *s) > > if (tp->cptse) { > css = props->ipcss; > -DBGOUT(TXSUM, "frames %d size %d ipcss %d\n", > - frames, tp->size, css); > +trace_e1000_xmit_seg1(frames, tp->size, css); > if (props->ip) {/* IPv4 */ > stw_be_p(tp->data+css+2, tp->size - css); >
Re: [PATCH v2] e1000: Convert debug macros into tracepoints.
On 4/3/24 03:45, Don Porter wrote: From: Austin Clements The E1000 debug messages are very useful for developing drivers. Make these available to users without recompiling QEMU. Signed-off-by: Austin Clements [geo...@ldpreload.com: Rebased on top of 2.9.0] Signed-off-by: Geoffrey Thomas Signed-off-by: Don Porter --- hw/net/e1000.c | 90 +++-- hw/net/trace-events | 25 - 2 files changed, 54 insertions(+), 61 deletions(-) Reviewed-by: Richard Henderson r~
[PATCH v2] e1000: Convert debug macros into tracepoints.
From: Austin Clements The E1000 debug messages are very useful for developing drivers. Make these available to users without recompiling QEMU. Signed-off-by: Austin Clements [geo...@ldpreload.com: Rebased on top of 2.9.0] Signed-off-by: Geoffrey Thomas Signed-off-by: Don Porter --- hw/net/e1000.c | 90 +++-- hw/net/trace-events | 25 - 2 files changed, 54 insertions(+), 61 deletions(-) diff --git a/hw/net/e1000.c b/hw/net/e1000.c index 43f3a4a701..24475636a3 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -44,26 +44,6 @@ #include "trace.h" #include "qom/object.h" -/* #define E1000_DEBUG */ - -#ifdef E1000_DEBUG -enum { -DEBUG_GENERAL, DEBUG_IO, DEBUG_MMIO, DEBUG_INTERRUPT, -DEBUG_RX, DEBUG_TX, DEBUG_MDIC, DEBUG_EEPROM, -DEBUG_UNKNOWN, DEBUG_TXSUM,DEBUG_TXERR,DEBUG_RXERR, -DEBUG_RXFILTER, DEBUG_PHY, DEBUG_NOTYET, -}; -#define DBGBIT(x)(1mac_reg[IMS]); +trace_e1000_set_ics(val, s->mac_reg[ICR], s->mac_reg[IMS]); set_interrupt_cause(s, 0, val | s->mac_reg[ICR]); } @@ -425,8 +404,7 @@ set_rx_control(E1000State *s, int index, uint32_t val) s->mac_reg[RCTL] = val; s->rxbuf_size = e1000x_rxbufsize(val); s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1; -DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT], - s->mac_reg[RCTL]); +trace_e1000_set_rx_control(s->mac_reg[RDT], s->mac_reg[RCTL]); timer_mod(s->flush_queue_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000); } @@ -440,16 +418,16 @@ set_mdic(E1000State *s, int index, uint32_t val) if ((val & E1000_MDIC_PHY_MASK) >> E1000_MDIC_PHY_SHIFT != 1) // phy # val = s->mac_reg[MDIC] | E1000_MDIC_ERROR; else if (val & E1000_MDIC_OP_READ) { -DBGOUT(MDIC, "MDIC read reg 0x%x\n", addr); +trace_e1000_mdic_read_register(addr); if (!(phy_regcap[addr] & PHY_R)) { -DBGOUT(MDIC, "MDIC read reg %x unhandled\n", addr); +trace_e1000_mdic_read_register_unhandled(addr); val |= E1000_MDIC_ERROR; } else val = (val ^ data) | s->phy_reg[addr]; } else if (val & E1000_MDIC_OP_WRITE) { -DBGOUT(MDIC, "MDIC write reg 0x%x, value 0x%x\n", addr, data); +trace_e1000_mdic_write_register(addr, data); if (!(phy_regcap[addr] & PHY_W)) { -DBGOUT(MDIC, "MDIC write reg %x unhandled\n", addr); +trace_e1000_mdic_write_register_unhandled(addr); val |= E1000_MDIC_ERROR; } else { if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) { @@ -471,8 +449,8 @@ get_eecd(E1000State *s, int index) { uint32_t ret = E1000_EECD_PRES|E1000_EECD_GNT | s->eecd_state.old_eecd; -DBGOUT(EEPROM, "reading eeprom bit %d (reading %d)\n", - s->eecd_state.bitnum_out, s->eecd_state.reading); +trace_e1000_get_eecd(s->eecd_state.bitnum_out, s->eecd_state.reading); + if (!s->eecd_state.reading || ((s->eeprom_data[(s->eecd_state.bitnum_out >> 4) & 0x3f] >> ((s->eecd_state.bitnum_out & 0xf) ^ 0xf))) & 1) @@ -511,9 +489,8 @@ set_eecd(E1000State *s, int index, uint32_t val) s->eecd_state.reading = (((s->eecd_state.val_in >> 6) & 7) == EEPROM_READ_OPCODE_MICROWIRE); } -DBGOUT(EEPROM, "eeprom bitnum in %d out %d, reading %d\n", - s->eecd_state.bitnum_in, s->eecd_state.bitnum_out, - s->eecd_state.reading); +trace_e1000_set_eecd(s->eecd_state.bitnum_in, s->eecd_state.bitnum_out, + s->eecd_state.reading); } static uint32_t @@ -580,8 +557,7 @@ xmit_seg(E1000State *s) if (tp->cptse) { css = props->ipcss; -DBGOUT(TXSUM, "frames %d size %d ipcss %d\n", - frames, tp->size, css); +trace_e1000_xmit_seg1(frames, tp->size, css); if (props->ip) {/* IPv4 */ stw_be_p(tp->data+css+2, tp->size - css); stw_be_p(tp->data+css+4, @@ -591,7 +567,7 @@ xmit_seg(E1000State *s) } css = props->tucss; len = tp->size - css; -DBGOUT(TXSUM, "tcp %d tucss %d len %d\n", props->tcp, css, len); +trace_e1000_xmit_seg2(props->tcp, css, len); if (props->tcp) { sofar = frames * props->mss; stl_be_p(tp->data+css+4, ldl_be_p(tp->data+css+4)+sofar); /* seq */ @@ -759,7 +735,7 @@ start_xmit(E1000State *s) uint32_t tdh_start = s->mac_reg[TDH], cause = E1000_ICS_TXQE; if (!(s->mac_reg[TCTL] & E1000_TCTL_EN)) { -DBGOUT(TX, "tx disabled\n"); +trace_e1000_start_xmit_fail1(); return; } @@ -773,9 +749,9 @@ start_xmit(E1000State *s) sizeof(struct e1000_tx_desc) * s->mac_reg[TDH]; pci_dma_read(d, base, , sizeof(desc)); -DBGOUT(TX, "index %d: %p : %x %x\n", s->mac_reg[TDH],