[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access
|
From: |
Dmitry Fleytman |
|
Subject: |
Re: [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access |
|
Date: |
Tue, 12 Jan 2016 08:43:38 +0200 |
> On 12 Jan 2016, at 04:38 AM, Miao Yan <address@hidden> wrote:
>
> Turning debug printfs to trace points for register access
Hello Miao!
While I’m into adding trace points I don’t really like the decrease of logs
usability introduced by this patch.
Current code produces clear human readable log that allows to trace execution
without looking into tables of commands and BAR layout.
I’d say that every printout you removed should be replaced with a trace point.
Thanks,
Dmitry
>
> Signed-off-by: Miao Yan <address@hidden>
> ---
> hw/net/vmxnet3.c | 68 +++++++++-----------------------------------------------
> trace-events | 6 +++++
> 2 files changed, 16 insertions(+), 58 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 67abad3..e089037 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -32,6 +32,8 @@
> #include "vmxnet_tx_pkt.h"
> #include "vmxnet_rx_pkt.h"
>
> +#include "trace.h"
> +
> #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
> #define VMXNET3_MSIX_BAR_SIZE 0x2000
> #define MIN_BUF_SIZE 60
> @@ -1157,6 +1159,8 @@ vmxnet3_io_bar0_write(void *opaque, hwaddr addr,
> {
> VMXNET3State *s = opaque;
>
> + trace_vmxnet3_bar0_write(opaque, addr, val);
> +
> if (VMW_IS_MULTIREG_ADDR(addr, VMXNET3_REG_TXPROD,
> VMXNET3_DEVICE_MAX_TX_QUEUES, VMXNET3_REG_ALIGN)) {
> int tx_queue_idx =
> @@ -1171,9 +1175,6 @@ vmxnet3_io_bar0_write(void *opaque, hwaddr addr,
> VMXNET3_MAX_INTRS, VMXNET3_REG_ALIGN)) {
> int l = VMW_MULTIREG_IDX_BY_ADDR(addr, VMXNET3_REG_IMR,
> VMXNET3_REG_ALIGN);
> -
> - VMW_CBPRN("Interrupt mask for line %d written: 0x%" PRIx64, l, val);
> -
> vmxnet3_on_interrupt_mask_changed(s, l, val);
> return;
> }
> @@ -1184,9 +1185,6 @@ vmxnet3_io_bar0_write(void *opaque, hwaddr addr,
> VMXNET3_DEVICE_MAX_RX_QUEUES, VMXNET3_REG_ALIGN)) {
> return;
> }
> -
> - VMW_WRPRN("BAR0 unknown write [%" PRIx64 "] = %" PRIx64 ", size %d",
> - (uint64_t) addr, val, size);
> }
>
> static uint64_t
> @@ -1201,7 +1199,8 @@ vmxnet3_io_bar0_read(void *opaque, hwaddr addr,
> unsigned size)
> return s->interrupt_states[l].is_masked;
> }
>
> - VMW_CBPRN("BAR0 unknown read [%" PRIx64 "], size %d", addr, size);
> + trace_vmxnet3_bar0_read(opaque, addr, 0);
> +
> return 0;
> }
>
> @@ -1315,7 +1314,6 @@ static void vmxnet3_setup_rx_filtering(VMXNET3State *s)
> static uint32_t vmxnet3_get_interrupt_config(VMXNET3State *s)
> {
> uint32_t interrupt_mode = VMXNET3_IT_AUTO | (VMXNET3_IMM_AUTO << 2);
> - VMW_CFPRN("Interrupt config is 0x%X", interrupt_mode);
> return interrupt_mode;
> }
>
> @@ -1614,85 +1612,66 @@ static void vmxnet3_handle_command(VMXNET3State *s,
> uint64_t cmd)
>
> switch (cmd) {
> case VMXNET3_CMD_GET_PERM_MAC_HI:
> - VMW_CBPRN("Set: Get upper part of permanent MAC");
> break;
>
> case VMXNET3_CMD_GET_PERM_MAC_LO:
> - VMW_CBPRN("Set: Get lower part of permanent MAC");
> break;
>
> case VMXNET3_CMD_GET_STATS:
> - VMW_CBPRN("Set: Get device statistics");
> vmxnet3_fill_stats(s);
> break;
>
> case VMXNET3_CMD_ACTIVATE_DEV:
> - VMW_CBPRN("Set: Activating vmxnet3 device");
> vmxnet3_activate_device(s);
> break;
>
> case VMXNET3_CMD_UPDATE_RX_MODE:
> - VMW_CBPRN("Set: Update rx mode");
> vmxnet3_update_rx_mode(s);
> break;
>
> case VMXNET3_CMD_UPDATE_VLAN_FILTERS:
> - VMW_CBPRN("Set: Update VLAN filters");
> vmxnet3_update_vlan_filters(s);
> break;
>
> case VMXNET3_CMD_UPDATE_MAC_FILTERS:
> - VMW_CBPRN("Set: Update MAC filters");
> vmxnet3_update_mcast_filters(s);
> break;
>
> case VMXNET3_CMD_UPDATE_FEATURE:
> - VMW_CBPRN("Set: Update features");
> vmxnet3_update_features(s);
> break;
>
> case VMXNET3_CMD_UPDATE_PMCFG:
> - VMW_CBPRN("Set: Update power management config");
> vmxnet3_update_pm_state(s);
> break;
>
> case VMXNET3_CMD_GET_LINK:
> - VMW_CBPRN("Set: Get link");
> break;
>
> case VMXNET3_CMD_RESET_DEV:
> - VMW_CBPRN("Set: Reset device");
> vmxnet3_reset(s);
> break;
>
> case VMXNET3_CMD_QUIESCE_DEV:
> - VMW_CBPRN("Set: VMXNET3_CMD_QUIESCE_DEV - deactivate the device");
> vmxnet3_deactivate_device(s);
> break;
>
> case VMXNET3_CMD_GET_CONF_INTR:
> - VMW_CBPRN("Set: VMXNET3_CMD_GET_CONF_INTR - interrupt
> configuration");
> break;
>
> case VMXNET3_CMD_GET_ADAPTIVE_RING_INFO:
> - VMW_CBPRN("Set: VMXNET3_CMD_GET_ADAPTIVE_RING_INFO - "
> - "adaptive ring info flags");
> break;
>
> case VMXNET3_CMD_GET_DID_LO:
> - VMW_CBPRN("Set: Get lower part of device ID");
> break;
>
> case VMXNET3_CMD_GET_DID_HI:
> - VMW_CBPRN("Set: Get upper part of device ID");
> break;
>
> case VMXNET3_CMD_GET_DEV_EXTRA_INFO:
> - VMW_CBPRN("Set: Get device extra info");
> break;
>
> default:
> - VMW_CBPRN("Received unknown command: %" PRIx64, cmd);
> break;
> }
> }
> @@ -1704,7 +1683,6 @@ static uint64_t vmxnet3_get_command_status(VMXNET3State
> *s)
> switch (s->last_command) {
> case VMXNET3_CMD_ACTIVATE_DEV:
> ret = (s->device_active) ? 0 : 1;
> - VMW_CFPRN("Device active: %" PRIx64, ret);
> break;
>
> case VMXNET3_CMD_RESET_DEV:
> @@ -1716,7 +1694,6 @@ static uint64_t vmxnet3_get_command_status(VMXNET3State
> *s)
>
> case VMXNET3_CMD_GET_LINK:
> ret = s->link_status_and_speed;
> - VMW_CFPRN("Link and speed: %" PRIx64, ret);
> break;
>
> case VMXNET3_CMD_GET_PERM_MAC_LO:
> @@ -1744,7 +1721,6 @@ static uint64_t vmxnet3_get_command_status(VMXNET3State
> *s)
> break;
>
> default:
> - VMW_WRPRN("Received request for unknown command: %x",
> s->last_command);
> ret = 0;
> break;
> }
> @@ -1765,7 +1741,6 @@ static void vmxnet3_ack_events(VMXNET3State *s,
> uint32_t val)
> {
> uint32_t events;
>
> - VMW_CBPRN("Clearing events: 0x%x", val);
> events = VMXNET3_READ_DRV_SHARED32(s->drv_shmem, ecr) & ~val;
> VMXNET3_WRITE_DRV_SHARED32(s->drv_shmem, ecr, events);
> }
> @@ -1778,23 +1753,19 @@ vmxnet3_io_bar1_write(void *opaque,
> {
> VMXNET3State *s = opaque;
>
> + trace_vmxnet3_bar1_write(opaque, addr, val);
> +
> switch (addr) {
> /* Vmxnet3 Revision Report Selection */
> case VMXNET3_REG_VRRS:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_VRRS] = %" PRIx64 ", size %d",
> - val, size);
> break;
>
> /* UPT Version Report Selection */
> case VMXNET3_REG_UVRS:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_UVRS] = %" PRIx64 ", size %d",
> - val, size);
> break;
>
> /* Driver Shared Address Low */
> case VMXNET3_REG_DSAL:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_DSAL] = %" PRIx64 ", size %d",
> - val, size);
> /*
> * Guest driver will first write the low part of the shared
> * memory address. We save it to temp variable and set the
> @@ -1809,8 +1780,6 @@ vmxnet3_io_bar1_write(void *opaque,
>
> /* Driver Shared Address High */
> case VMXNET3_REG_DSAH:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_DSAH] = %" PRIx64 ", size %d",
> - val, size);
> /*
> * Set the shared memory between guest driver and device.
> * We already should have low address part.
> @@ -1820,42 +1789,30 @@ vmxnet3_io_bar1_write(void *opaque,
>
> /* Command */
> case VMXNET3_REG_CMD:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_CMD] = %" PRIx64 ", size %d",
> - val, size);
> vmxnet3_handle_command(s, val);
> break;
>
> /* MAC Address Low */
> case VMXNET3_REG_MACL:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_MACL] = %" PRIx64 ", size %d",
> - val, size);
> s->temp_mac = val;
> break;
>
> /* MAC Address High */
> case VMXNET3_REG_MACH:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_MACH] = %" PRIx64 ", size %d",
> - val, size);
> vmxnet3_set_variable_mac(s, val, s->temp_mac);
> break;
>
> /* Interrupt Cause Register */
> case VMXNET3_REG_ICR:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_ICR] = %" PRIx64 ", size %d",
> - val, size);
> g_assert_not_reached();
> break;
>
> /* Event Cause Register */
> case VMXNET3_REG_ECR:
> - VMW_CBPRN("Write BAR1 [VMXNET3_REG_ECR] = %" PRIx64 ", size %d",
> - val, size);
> vmxnet3_ack_events(s, val);
> break;
>
> default:
> - VMW_CBPRN("Unknown Write to BAR1 [%" PRIx64 "] = %" PRIx64 ", size
> %d",
> - addr, val, size);
> break;
> }
> }
> @@ -1869,31 +1826,26 @@ vmxnet3_io_bar1_read(void *opaque, hwaddr addr,
> unsigned size)
> switch (addr) {
> /* Vmxnet3 Revision Report Selection */
> case VMXNET3_REG_VRRS:
> - VMW_CBPRN("Read BAR1 [VMXNET3_REG_VRRS], size %d", size);
> ret = VMXNET3_DEVICE_REVISION;
> break;
>
> /* UPT Version Report Selection */
> case VMXNET3_REG_UVRS:
> - VMW_CBPRN("Read BAR1 [VMXNET3_REG_UVRS], size %d", size);
> ret = VMXNET3_UPT_REVISION;
> break;
>
> /* Command */
> case VMXNET3_REG_CMD:
> - VMW_CBPRN("Read BAR1 [VMXNET3_REG_CMD], size %d", size);
> ret = vmxnet3_get_command_status(s);
> break;
>
> /* MAC Address Low */
> case VMXNET3_REG_MACL:
> - VMW_CBPRN("Read BAR1 [VMXNET3_REG_MACL], size %d", size);
> ret = vmxnet3_get_mac_low(&s->conf.macaddr);
> break;
>
> /* MAC Address High */
> case VMXNET3_REG_MACH:
> - VMW_CBPRN("Read BAR1 [VMXNET3_REG_MACH], size %d", size);
> ret = vmxnet3_get_mac_high(&s->conf.macaddr);
> break;
>
> @@ -1902,7 +1854,6 @@ vmxnet3_io_bar1_read(void *opaque, hwaddr addr,
> unsigned size)
> * Used for legacy interrupts only so interrupt index always 0
> */
> case VMXNET3_REG_ICR:
> - VMW_CBPRN("Read BAR1 [VMXNET3_REG_ICR], size %d", size);
> if (vmxnet3_interrupt_asserted(s, 0)) {
> vmxnet3_clear_interrupt(s, 0);
> ret = true;
> @@ -1912,10 +1863,11 @@ vmxnet3_io_bar1_read(void *opaque, hwaddr addr,
> unsigned size)
> break;
>
> default:
> - VMW_CBPRN("Unknow read BAR1[%" PRIx64 "], %d bytes", addr, size);
> break;
> }
>
> + trace_vmxnet3_bar1_read(opaque, addr, ret);
> +
> return ret;
> }
>
> diff --git a/trace-events b/trace-events
> index 6f03638..48323e2 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1375,6 +1375,12 @@ pcnet_mmio_readb(void *opaque, uint64_t addr, uint32_t
> val) "opaque=%p addr=%#"P
> pcnet_mmio_readw(void *opaque, uint64_t addr, uint32_t val) "opaque=%p
> addr=%#"PRIx64" val=0x%x"
> pcnet_mmio_readl(void *opaque, uint64_t addr, uint32_t val) "opaque=%p
> addr=%#"PRIx64" val=0x%x"
>
> +# hw/net/vmxnet3.c
> +vmxnet3_bar0_write(void *opaque, uint64_t addr, uint64_t val) "opaque=%p
> addr=0x%"PRIx64" val=0x%"PRIx64
> +vmxnet3_bar0_read(void *opaque, uint64_t addr, uint64_t val) "opaque=%p
> addr=0x%"PRIx64" val=0x%"PRIx64
> +vmxnet3_bar1_write(void *opaque, uint64_t addr, uint64_t val) "opaque=%p
> addr=0x%"PRIx64" val=0x%"PRIx64
> +vmxnet3_bar1_read(void *opaque, uint64_t addr, uint64_t val) "opaque=%p
> addr=0x%"PRIx64" val=0x%"PRIx64
> +
> # hw/intc/xics.c
> xics_icp_check_ipi(int server, uint8_t mfrr) "CPU %d can take IPI mfrr=%#x"
> xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr) "icp_accept: XIRR
> %#"PRIx32"->%#"PRIx32
> --
> 1.9.1
>