Re: [Qemu-devel] [RFC PATCH v5 07/31] icount: implement icount requesting

[Paolo Bonzini]

On 05/12/2014 13:59, Pavel Dovgaluk wrote:
>> From: Paolo Bonzini [mailto:address@hidden
>> On 05/12/2014 11:55, Pavel Dovgaluk wrote:
>>>>>
>>>>> And why is can_do_io zero? :)  Is the fix to move the place where
>>>>> can_do_io becomes nonzero?
>>> can_do_io is set by gen_io_start function.
>>> As I understand, it is used to protect determinism in icount mode,
>>> because it allows non-deterministic (port io, raising interrupt)
>>> operations only at the end of the translation blocks.
>>> When someone tries to use MMIO in the middle of TB, that TB is
>>> recompiled to place this instruction at the end of the block.
>>>
>>> Do you mean that we can set can_do_io before execution of the block
>>> and reset it at the beginning of the execution?
>>
>> Yes, we could try setting it after execution of the block and clearing
>> it afterwards.  Peter knows that part of icount better though (I know
>> mostly the timer/warping parts).
> 
> Ok, how about these changes?
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index f52f292..88675ca 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -168,7 +168,9 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, 
> uint8_t *tb_ptr)
>      }
>  #endif /* DEBUG_DISAS */
>  
> +    cpu->can_do_io = 0;
>      next_tb = tcg_qemu_tb_exec(env, tb_ptr);
> +    cpu->can_do_io = 1;
>      trace_exec_tb_exit((void *) (next_tb & ~TB_EXIT_MASK),
>                         next_tb & TB_EXIT_MASK);
>  
> @@ -548,6 +550,7 @@ int cpu_exec(CPUArchState *env)
>              cpu = current_cpu;
>              env = cpu->env_ptr;
>              cc = CPU_GET_CLASS(cpu);
> +            cpu->can_do_io = 1;
>  #ifdef TARGET_I386
>              x86_cpu = X86_CPU(cpu);
>  #endif
> diff --git a/cpus.c b/cpus.c
> index 0c33458..7a45a51 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -934,6 +934,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>      qemu_mutex_lock(&qemu_global_mutex);
>      qemu_thread_get_self(cpu->thread);
>      cpu->thread_id = qemu_get_thread_id();
> +    cpu->can_do_io = 1;
>      current_cpu = cpu;
>  
>      r = kvm_init_vcpu(cpu);
> @@ -974,6 +975,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
>      qemu_mutex_lock_iothread();
>      qemu_thread_get_self(cpu->thread);
>      cpu->thread_id = qemu_get_thread_id();
> +    cpu->can_do_io = 1;
>  
>      sigemptyset(&waitset);
>      sigaddset(&waitset, SIG_IPI);
> @@ -1016,6 +1018,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>      CPU_FOREACH(cpu) {
>          cpu->thread_id = qemu_get_thread_id();
>          cpu->created = true;
> +        cpu->can_do_io = 1;
>      }
>      qemu_cond_signal(&qemu_cpu_cond);

Yes, this would work too, thanks for trying it!

Paolo