* [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access
@ 2016-01-12 2:38 Miao Yan
2016-01-12 6:43 ` Dmitry Fleytman
0 siblings, 1 reply; 5+ messages in thread
From: Miao Yan @ 2016-01-12 2:38 UTC (permalink / raw)
To: jasowang, dmitry, qemu-devel
Turning debug printfs to trace points for register access
Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access
2016-01-12 2:38 [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access Miao Yan
@ 2016-01-12 6:43 ` Dmitry Fleytman
2016-01-12 7:23 ` Miao Yan
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Fleytman @ 2016-01-12 6:43 UTC (permalink / raw)
To: Miao Yan; +Cc: jasowang, qemu-devel
> On 12 Jan 2016, at 04:38 AM, Miao Yan <yanmiaobest@gmail.com> 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 <yanmiaobest@gmail.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access
2016-01-12 6:43 ` Dmitry Fleytman
@ 2016-01-12 7:23 ` Miao Yan
2016-01-12 7:57 ` Dmitry Fleytman
0 siblings, 1 reply; 5+ messages in thread
From: Miao Yan @ 2016-01-12 7:23 UTC (permalink / raw)
To: Dmitry Fleytman; +Cc: Jason Wang, QEMU
Hi Dmitry,
2016-01-12 14:43 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>:
>
>> On 12 Jan 2016, at 04:38 AM, Miao Yan <yanmiaobest@gmail.com> 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.
How about I add trace point and keep those debug logs ?
> 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.
The printfs that I removed are only for register accesses, which are already
covered by trace. I didn't touch others in the code flow.
Thanks,
Miao
>
> Thanks,
> Dmitry
>
>>
>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>> ---
>> 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
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access
2016-01-12 7:23 ` Miao Yan
@ 2016-01-12 7:57 ` Dmitry Fleytman
2016-01-12 8:59 ` Miao Yan
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Fleytman @ 2016-01-12 7:57 UTC (permalink / raw)
To: Miao Yan; +Cc: Jason Wang, QEMU
> On 12 Jan 2016, at 09:23 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>
> Hi Dmitry,
>
> 2016-01-12 14:43 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>:
>>
>>> On 12 Jan 2016, at 04:38 AM, Miao Yan <yanmiaobest@gmail.com> 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.
>
> How about I add trace point and keep those debug logs ?
I’d prefer the complete solution i.e. to replace all printouts with traces. Otherwise it doesn’t make much sense.
>
>> 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.
>
> The printfs that I removed are only for register accesses, which are already
> covered by trace. I didn't touch others in the code flow.
I understand this. My point is that generic trace is less readable than a number of specific ones, so do not drop specific printouts, convert those to trace points.
>
> Thanks,
> Miao
>
>
>>
>> Thanks,
>> Dmitry
>>
>>>
>>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>>> ---
>>> 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
>>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access
2016-01-12 7:57 ` Dmitry Fleytman
@ 2016-01-12 8:59 ` Miao Yan
0 siblings, 0 replies; 5+ messages in thread
From: Miao Yan @ 2016-01-12 8:59 UTC (permalink / raw)
To: Dmitry Fleytman; +Cc: Jason Wang, QEMU
2016-01-12 15:57 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>:
>
>> On 12 Jan 2016, at 09:23 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>>
>> Hi Dmitry,
>>
>> 2016-01-12 14:43 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>:
>>>
>>>> On 12 Jan 2016, at 04:38 AM, Miao Yan <yanmiaobest@gmail.com> 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.
>>
>> How about I add trace point and keep those debug logs ?
>
> I’d prefer the complete solution i.e. to replace all printouts with traces. Otherwise it doesn’t make much sense.
OK, I get your point. Will investigate and prepare v2. Thanks.
>
>>
>>> 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.
>>
>> The printfs that I removed are only for register accesses, which are already
>> covered by trace. I didn't touch others in the code flow.
>
> I understand this. My point is that generic trace is less readable than a number of specific ones, so do not drop specific printouts, convert those to trace points.
>
>>
>> Thanks,
>> Miao
>>
>>
>>>
>>> Thanks,
>>> Dmitry
>>>
>>>>
>>>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>>>> ---
>>>> 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
>>>>
>>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-01-12 8:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-12 2:38 [Qemu-devel] [PATCH] net/vmxnet3: trace support for register access Miao Yan
2016-01-12 6:43 ` Dmitry Fleytman
2016-01-12 7:23 ` Miao Yan
2016-01-12 7:57 ` Dmitry Fleytman
2016-01-12 8:59 ` Miao Yan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.