Re: [Qemu-devel] [RFC PATCH V7 07/19] protect TBContext with tb_lock.

[Alex Bennée]

Frederic Konrad <address@hidden> writes:

> On 10/08/2015 17:27, address@hidden wrote:
>> From: KONRAD Frederic <address@hidden>
>>
>> This protects TBContext with tb_lock to make tb_* thread safe.
>>
>> We can still have issue with tb_flush in case of multithread TCG:
>>    An other CPU can be executing code during a flush.
>>
>> This can be fixed later by making all other TCG thread exiting before calling
>> tb_flush().
>>
>> tb_find_slow is separated into tb_find_slow and tb_find_physical as the whole
>> tb_find_slow doesn't require to lock the tb.
>>
>> Signed-off-by: KONRAD Frederic <address@hidden>
>>
>> Changes:
> [...]
>>   
>> @@ -675,6 +710,7 @@ static inline void code_gen_alloc(size_t tb_size)
>>               CODE_GEN_AVG_BLOCK_SIZE;
>>       tcg_ctx.tb_ctx.tbs =
>>               g_malloc(tcg_ctx.code_gen_max_blocks * 
>> sizeof(TranslationBlock));
>> +    qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
>>   }
>>   
>>   /* Must be called before using the QEMU cpus. 'tb_size' is the size
>> @@ -699,16 +735,22 @@ bool tcg_enabled(void)
>>       return tcg_ctx.code_gen_buffer != NULL;
>>   }
>>   
>> -/* Allocate a new translation block. Flush the translation buffer if
>> -   too many translation blocks or too much generated code. */
>> +/*
>> + * Allocate a new translation block. Flush the translation buffer if
>> + * too many translation blocks or too much generated code.
>> + * tb_alloc is not thread safe but tb_gen_code is protected by a mutex so 
>> this
>> + * function is called only by one thread.
>> + */
>>   static TranslationBlock *tb_alloc(target_ulong pc)
>>   {
>> -    TranslationBlock *tb;
>> +    TranslationBlock *tb = NULL;
>>   
>>       if (tcg_ctx.tb_ctx.nb_tbs >= tcg_ctx.code_gen_max_blocks ||
>>           (tcg_ctx.code_gen_ptr - tcg_ctx.code_gen_buffer) >=
>>            tcg_ctx.code_gen_buffer_max_size) {
>> -        return NULL;
>> +        tb = &tcg_ctx.tb_ctx.tbs[tcg_ctx.tb_ctx.nb_tbs++];
>> +        tb->pc = pc;
>> +        tb->cflags = 0;
>
> Missed this wrong unreverted part which in the end doesn't do a tb_flush 
> when required and crashes!
> Fixing that allows me to boot with jessie and virt.

\o/

Do you see crashes while it is running?

It's interesting that I've not had a problem booting jessie with virt
though - just crashes while hanging.

Are you likely to push a v8 this week (or a temp branch?) with this and
any other obvious fixes? I appreciate Paolo has given you a not-so-small
pile of review comments as well so I wasn't looking for a complete new
patch set!


>
> Fred

-- 
Alex Bennée