Re: [Qemu-devel] [PATCH 20/32] accel_trace_io

[Glauber Costa]

On Sat, Oct 25, 2008 at 9:10 AM, andrzej zaborowski <address@hidden> wrote:
> 2008/10/23 Glauber Costa <address@hidden>:
>> From: Glauber Costa <address@hidden>
>>
>> kqemu keeps trace of the last io done. Do it through
>> an accel_wrapper.
>>
>> Signed-off-by: Glauber Costa <address@hidden>
>> ---
>>  accel.c            |    2 ++
>>  accel.h            |   13 ++++++++++++-
>>  cpu-exec.c         |    9 ++-------
>>  kqemu.c            |   18 ++++++++++++++++++
>>  softmmu_template.h |   10 ++++------
>>  vl.c               |   30 ++++++------------------------
>>  6 files changed, 44 insertions(+), 38 deletions(-)
>>
>> diff --git a/accel.c b/accel.c
>> index bf1194d..0890039 100644
>> --- a/accel.c
>> +++ b/accel.c
>> @@ -32,5 +32,7 @@ QEMUAccel noaccel = {
>>     .get_real_ticks = cpu_get_ticks,
>>  #endif
>>     .register_physical_memory = accel_nop,
>> +    .trace_io = accel_nop,
>> +    .break_loop = accel_nop,
>>  };
>>
>> diff --git a/accel.h b/accel.h
>> index dbb6372..b37175b 100644
>> --- a/accel.h
>> +++ b/accel.h
>> @@ -19,7 +19,8 @@ typedef struct QEMUAccel {
>>  #endif
>>     void (*register_physical_memory)(uint64_t start_addr,
>>                                      ram_addr_t size, ram_addr_t 
>> phys_offset);
>> -
>> +    void (*trace_io)(CPUState *env);
>> +    int (*break_loop)(CPUState *env);
>>  } QEMUAccel;
>>
>>  typedef struct QEMUCont {
>> @@ -142,4 +143,14 @@ static inline void accel_register_phys_mem(uint64_t 
>> start_addr,
>>  {
>>     current_accel->register_physical_memory(start_addr, size, phys_offset);
>>  }
>> +
>> +static inline void accel_trace_io(CPUState *env)
>> +{
>> +    current_accel->trace_io(env);
>> +}
>> +
>> +static inline int accel_break_loop(CPUState *env)
>> +{
>> +    return current_accel->break_loop(env);
>> +}
>>  #endif
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index 88b7d6f..18908d5 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -36,6 +36,7 @@
>>  #include <signal.h>
>>  #include <sys/ucontext.h>
>>  #endif
>> +#include "accel.h"
>>
>>  #if defined(__sparc__) && !defined(HOST_SOLARIS)
>>  // Work around ugly bugs in glibc that mangle global register contents
>> @@ -646,13 +647,7 @@ int cpu_exec(CPUState *env1)
>>                 }
>>                 /* reset soft MMU for next block (it can currently
>>                    only be set by a memory fault) */
>> -#if defined(USE_KQEMU)
>> -#define MIN_CYCLE_BEFORE_SWITCH (100 * 1000)
>> -                if (kqemu_is_ok(env) &&
>> -                    (cpu_get_time_fast() - env->last_io_time) >= 
>> MIN_CYCLE_BEFORE_SWITCH) {
>> -                    cpu_loop_exit();
>> -                }
>> -#endif
>> +                accel_break_loop(env);
>>             } /* for(;;) */
>>         } else {
>>             env_to_regs();
>> diff --git a/kqemu.c b/kqemu.c
>> index f4d905a..d38d0e3 100644
>> --- a/kqemu.c
>> +++ b/kqemu.c
>> @@ -1107,6 +1107,22 @@ static int kqemu_profile(CPUState *env, char *buf)
>>     return len;
>>  }
>>
>> +static void kqemu_trace_io(CPUState *env)
>> +{
>> +    if (env)
>> +        env->last_io_time = cpu_get_time_fast();
>> +}
>> +
>> +static int kqemu_break_loop(CPUState *env)
>> +{
>> +#define MIN_CYCLE_BEFORE_SWITCH (100 * 1000)
>> +    if (kqemu_is_ok(env) &&
>> +        (cpu_get_time_fast() - env->last_io_time) >= 
>> MIN_CYCLE_BEFORE_SWITCH) {
>> +        return 1;
>> +    }
>> +    return 0;
>> +}
>> +
>>  QEMUAccel kqemu_accel = {
>>     .name = "KQEMU",
>>     .cpu_interrupt = kqemu_cpu_interrupt,
>> @@ -1125,6 +1141,8 @@ QEMUAccel kqemu_accel = {
>>     .get_real_ticks = cpu_get_real_ticks,
>>  #endif
>>     .register_physical_memory = kqemu_set_phys_mem,
>> +    .trace_io = kqemu_trace_io,
>> +    .break_loop = kqemu_break_loop,
>>  };
>>
>>  #endif
>> diff --git a/softmmu_template.h b/softmmu_template.h
>> index 98dd378..4945352 100644
>> --- a/softmmu_template.h
>> +++ b/softmmu_template.h
>> @@ -47,6 +47,8 @@
>>  #define ADDR_READ addr_read
>>  #endif
>>
>> +#include "accel.h"
>> +
>>  static DATA_TYPE glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(target_ulong addr,
>>                                                         int mmu_idx,
>>                                                         void *retaddr);
>> @@ -75,9 +77,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
>> +    accel_trace_io(env);
>>     return res;
>>  }
>>
>> @@ -220,9 +220,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
>> +    accel_trace_io(env);
>>  }
>>
>>  void REGPARM glue(glue(__st, SUFFIX), MMUSUFFIX)(target_ulong addr,
>> diff --git a/vl.c b/vl.c
>> index 964205d..e64becb 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -420,10 +420,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
>> +    accel_trace_io(env);
>>  }
>>
>>  void cpu_outw(CPUState *env, int addr, int val)
>> @@ -433,10 +430,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
>> +    accel_trace_io(env);
>>  }
>>
>>  void cpu_outl(CPUState *env, int addr, int val)
>> @@ -446,10 +440,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
>> +    accel_trace_io(env);
>>  }
>>
>>  int cpu_inb(CPUState *env, int addr)
>> @@ -460,10 +451,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
>> +    accel_trace_io(env);
>>     return val;
>>  }
>>
>> @@ -475,10 +463,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
>> +    accel_trace_io(env);
>>     return val;
>>  }
>>
>> @@ -490,10 +475,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
>> +    accel_trace_io(env);
>
> It would be great if all the new checks on the critical paths could
> remain wrapped in some kind of #ifdef for archs that don't have any
> acceleration implemented (also in other patches in this series).

Yeah, me and anthony were discussing exactly that, yesterday.
It's part of the plan now.



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."