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

[Frederic Konrad <fred.konrad@greensocs.com>]

On 12/08/2015 20:20, Alex Bennée wrote:
> Frederic Konrad <fred.konrad@greensocs.com> writes:
>
>> On 10/08/2015 17:27, fred.konrad@greensocs.com wrote:
>>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>>
>>> 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 <fred.konrad@greensocs.com>
>>>
>>> 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!
here is something I did yesterday:
multi_tcg_v7_bugfixed

The patch-set is a mess and not re-based on the patch-set sent by Paolo.

Fred

>
>> Fred