[Qemu-devel] [PATCH 0/2] Add generic infrastructure to trace I/O

[Qemu-devel] [PATCH 0/2] Add generic infrastructure to trace I/O

[Glauber Costa]

Hi guys,

Based on the discussions we had yesterday about the accel, me and anthony
had discussions that led us to think that the best way to avoid accel
hooks in the io operations is to generically keep track of I/O that 
happens throughout the execution. To demonstrate how this can be useful
besides the accelerator scope, I wrote an info iostats monitor command.

Comments welcome

[Qemu-devel] [PATCH 1/2] trace io operations

[Glauber Costa]

Add fields in cpu state to trace reads and writes for both
pio and mmio. Together with it, keep track of the timestamp
in which the last io happened, whatever kind it was. This last
part is particularly useful for us to replace kqemu break-loop
tests by a generic framework without the need for an accelerator
hook.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 cpu-defs.h         |    6 ++++++
 softmmu_template.h |    8 ++------
 target-i386/cpu.h  |    1 -
 vl.c               |   50 ++++++++++++++++++++++++++------------------------
 4 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index 5dcac74..4b401ef 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -153,6 +153,12 @@ typedef struct icount_decr_u16 {
                                 accessed */                             \
     target_ulong mem_io_vaddr; /* target virtual addr at which the      \
                                      memory was accessed */             \
+    uint32_t mmio_read_count;                                                \
+    uint32_t pio_read_count;                                                 \
+    uint32_t mmio_write_count;                                                \
+    uint32_t pio_write_count;                                                 \
+    uint64_t last_io_time;                                              \
+                                                                        \
     uint32_t halted; /* Nonzero if the CPU is in suspend state */       \
     uint32_t interrupt_request;                                         \
     /* The meaning of the MMU modes is defined in the target code. */   \
diff --git a/softmmu_template.h b/softmmu_template.h
index 98dd378..cdfcb38 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -75,9 +75,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(target_phys_addr_t physaddr,
     res |= (uint64_t)io_mem_read[index][2](io_mem_opaque[index], physaddr + 4) << 32;
 #endif
 #endif /* SHIFT > 2 */
-#ifdef USE_KQEMU
-    env->last_io_time = cpu_get_time_fast();
-#endif
+    trace_mmio(env, 0);
     return res;
 }
 
@@ -220,9 +218,7 @@ static inline void glue(io_write, SUFFIX)(target_phys_addr_t physaddr,
     io_mem_write[index][2](io_mem_opaque[index], physaddr + 4, val >> 32);
 #endif
 #endif /* SHIFT > 2 */
-#ifdef USE_KQEMU
-    env->last_io_time = cpu_get_time_fast();
-#endif
+    trace_mmio(env, 1);
 }
 
 void REGPARM glue(glue(__st, SUFFIX), MMUSUFFIX)(target_ulong addr,
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 3c11e0f..8a2d797 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -608,7 +608,6 @@ typedef struct CPUX86State {
 
 #ifdef USE_KQEMU
     int kqemu_enabled;
-    int last_io_time;
 #endif
     /* in order to simplify APIC support, we leave this pointer to the
        user */
diff --git a/vl.c b/vl.c
index 16d3e96..dfec9ff 100644
--- a/vl.c
+++ b/vl.c
@@ -263,6 +263,26 @@ PicState2 *isa_pic;
 static IOPortReadFunc default_ioport_readb, default_ioport_readw, default_ioport_readl;
 static IOPortWriteFunc default_ioport_writeb, default_ioport_writew, default_ioport_writel;
 
+void trace_pio(CPUState *env, int write)
+{
+    if (write)
+        env->pio_write_count++;
+    else
+        env->pio_read_count++;
+
+    env->last_io_time = cpu_get_real_ticks();
+}
+
+void trace_mmio(CPUState *env, int write)
+{
+    if (write)
+        env->mmio_write_count++;
+    else
+        env->mmio_read_count++;
+
+    env->last_io_time = cpu_get_real_ticks();
+}
+
 static void qemu_io_interrupt(CPUState *env)
 {
     if (env) {
@@ -422,10 +442,7 @@ void cpu_outb(CPUState *env, int addr, int val)
         fprintf(logfile, "outb: %04x %02x\n", addr, val);
 #endif
     ioport_write(0, addr, val);
-#ifdef USE_KQEMU
-    if (env)
-        env->last_io_time = cpu_get_time_fast();
-#endif
+    trace_pio(env, 1);
 }
 
 void cpu_outw(CPUState *env, int addr, int val)
@@ -435,10 +452,7 @@ void cpu_outw(CPUState *env, int addr, int val)
         fprintf(logfile, "outw: %04x %04x\n", addr, val);
 #endif
     ioport_write(1, addr, val);
-#ifdef USE_KQEMU
-    if (env)
-        env->last_io_time = cpu_get_time_fast();
-#endif
+    trace_pio(env, 1);
 }
 
 void cpu_outl(CPUState *env, int addr, int val)
@@ -448,10 +462,7 @@ void cpu_outl(CPUState *env, int addr, int val)
         fprintf(logfile, "outl: %04x %08x\n", addr, val);
 #endif
     ioport_write(2, addr, val);
-#ifdef USE_KQEMU
-    if (env)
-        env->last_io_time = cpu_get_time_fast();
-#endif
+    trace_pio(env, 1);
 }
 
 int cpu_inb(CPUState *env, int addr)
@@ -462,10 +473,7 @@ int cpu_inb(CPUState *env, int addr)
     if (loglevel & CPU_LOG_IOPORT)
         fprintf(logfile, "inb : %04x %02x\n", addr, val);
 #endif
-#ifdef USE_KQEMU
-    if (env)
-        env->last_io_time = cpu_get_time_fast();
-#endif
+    trace_pio(env, 0);
     return val;
 }
 
@@ -477,10 +485,7 @@ int cpu_inw(CPUState *env, int addr)
     if (loglevel & CPU_LOG_IOPORT)
         fprintf(logfile, "inw : %04x %04x\n", addr, val);
 #endif
-#ifdef USE_KQEMU
-    if (env)
-        env->last_io_time = cpu_get_time_fast();
-#endif
+    trace_pio(env, 0);
     return val;
 }
 
@@ -492,10 +497,7 @@ int cpu_inl(CPUState *env, int addr)
     if (loglevel & CPU_LOG_IOPORT)
         fprintf(logfile, "inl : %04x %08x\n", addr, val);
 #endif
-#ifdef USE_KQEMU
-    if (env)
-        env->last_io_time = cpu_get_time_fast();
-#endif
+    trace_pio(env, 0);
     return val;
 }
 
-- 
1.5.5.1

[Qemu-devel] [PATCH 2/2] new monitor command: info iostats

[Glauber Costa]

This command provides per-cpu statistics on I/O that
has taken place in this cpu since the last time the command
was issued. We track reads and writes for pio and mmio.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 monitor.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index f0a0bc3..f70dfe9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -291,6 +291,27 @@ static CPUState *mon_get_cpu(void)
     return mon_cpu;
 }
 
+static void do_info_iostats(void)
+{
+    uint32_t total_mmio = 0;
+    uint32_t total_pio = 0;
+    CPUState *env;
+
+    mon_get_cpu();
+
+    for(env = first_cpu; env != NULL; env = env->next_cpu) {
+        term_printf("CPU%d: pio_read=%d pio_write=%d mmio_read=%d mmio_write=%d\n",
+                    env->cpu_index, env->pio_read_count, env->pio_write_count,
+                    env->mmio_read_count, env->mmio_write_count);
+        total_mmio += env->mmio_read_count + env->mmio_write_count;
+        env->mmio_read_count = env->mmio_write_count = 0;
+        total_pio += env->pio_read_count + env->pio_write_count;
+        env->pio_read_count = env->pio_write_count = 0;
+    }
+    term_printf("total: pio=%d mmio=%d\n", total_pio, total_mmio);
+}
+
+
 static void do_info_registers(void)
 {
     CPUState *env;
@@ -1473,6 +1494,8 @@ static const term_cmd_t info_cmds[] = {
       "", "show the block devices" },
     { "blockstats", "", do_info_blockstats,
       "", "show block device statistics" },
+    { "iostats", "", do_info_iostats,
+      "", "show I/O statistics"},
     { "registers", "", do_info_registers,
       "", "show the cpu registers" },
     { "cpus", "", do_info_cpus,
-- 
1.5.5.1

[Qemu-devel] Re: [PATCH 2/2] new monitor command: info iostats

[Avi Kivity]

Glauber Costa wrote:
> This command provides per-cpu statistics on I/O that
> has taken place in this cpu since the last time the command
> was issued. We track reads and writes for pio and mmio.
>   

Best to always give out totals and let the user subtract if they want to.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH 1/2] trace io operations

[Avi Kivity]

Glauber Costa wrote:
>                                       memory was accessed */             \
> +    uint32_t mmio_read_count;                                                \
> +    uint32_t pio_read_count;                                                 \
> +    uint32_t mmio_write_count;                                                \
> +    uint32_t pio_write_count;                                                 \
>   

Will easily overflow.

-- 
error compiling committee.c: too many arguments to function