Re: [Qemu-devel] [PATCH 5/5] tcg: move tb_invalidated_flag to CPUState

[Sergey Fedorov <serge.fdrv@gmail.com>]

On 22/03/16 18:07, Alex Bennée wrote:
> sergey.fedorov@linaro.org writes:
(snip)
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index f90482eff778..07545aa91082 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -195,10 +195,11 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
>>      if (max_cycles > CF_COUNT_MASK)
>>          max_cycles = CF_COUNT_MASK;
>>
>> +    cpu->tb_invalidated_flag = 0;
> We've declared as bool so lets use true/false instead of 1/0's
>
(snip)
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index 7052eee7b78a..9538f9cc2af3 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -240,6 +240,7 @@ struct kvm_run;
>>   * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU
>>   * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this
>>   *           CPU and return to its top level loop.
>> + * @tb_invalidated_flag: Set to tell TCG that tb_flush has been called.
>>   * @singlestep_enabled: Flags for single-stepping.
>>   * @icount_extra: Instructions until next timer event.
>>   * @icount_decr: Number of cycles left, with interrupt flag in high bit.
>> @@ -291,6 +292,7 @@ struct CPUState {
>>      bool stopped;
>>      bool crash_occurred;
>>      bool exit_request;
>> +    bool tb_invalidated_flag;
> s/_flag// would save a few characters given it should be obvious it is a
> flag from the setting of true and false?
>
>>    
(snip)
>> diff --git a/translate-all.c b/translate-all.c
>> index 1db5a914d9a3..8e1edd6bb633 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -843,6 +843,7 @@ void tb_flush(CPUState *cpu)
>>      tcg_ctx.tb_ctx.nb_tbs = 0;
>>
>>      CPU_FOREACH(cpu) {
>> +        cpu->tb_invalidated_flag = 1;
>>          memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
>>      }
>>
>> @@ -1079,8 +1080,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>          /* cannot fail at this point */
>>          tb = tb_alloc(pc);
>>          assert(tb != NULL);
>> -        /* Don't forget to invalidate previous TB info.  */
>> -        tcg_ctx.tb_ctx.tb_invalidated_flag = 1;
>>      }
>>
>>      gen_code_buf = tcg_ctx.code_gen_ptr;
> I also note there is some code motion about where these flags are set
> and cleared which should probably be mentioned in the commit message.
>

Agree with all the comments.

Kind regards,
Sergey