[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v3 06/11] tcg: Introduce tb_mark_invalid() and tb_
From: |
Alex Bennée |
Subject: |
Re: [Qemu-ppc] [PATCH v3 06/11] tcg: Introduce tb_mark_invalid() and tb_is_invalid() |
Date: |
Thu, 14 Jul 2016 14:15:57 +0100 |
User-agent: |
mu4e 0.9.17; emacs 25.0.95.9 |
Sergey Fedorov <address@hidden> writes:
> On 14/07/16 15:53, Alex Bennée wrote:
>> Sergey Fedorov <address@hidden> writes:
>>
>>> From: Sergey Fedorov <address@hidden>
>>>
>>> These functions will be used to make translation block invalidation safe
>>> with concurrent lockless lookup in the global hash table.
>>>
>>> Most targets don't use 'cs_base'; so marking TB as invalid is as simple
>>> as assigning -1 to 'cs_base'. SPARC target stores the next program
>>> counter into 'cs_base', and -1 is a fine invalid value since PC must bet
>>> a multiple of 4 in SPARC. The only odd target is i386, for which a
>>> special flag is introduced in place of removed 'HF_SOFTMMU_MASK'.
>>>
>>> Suggested-by: Paolo Bonzini <address@hidden>
>>> Signed-off-by: Sergey Fedorov <address@hidden>
>>> Signed-off-by: Sergey Fedorov <address@hidden>
>>> ---
>>> include/exec/exec-all.h | 10 ++++++++++
>>> target-alpha/cpu.h | 14 ++++++++++++++
>>> target-arm/cpu.h | 14 ++++++++++++++
>>> target-cris/cpu.h | 14 ++++++++++++++
>>> target-i386/cpu.h | 17 +++++++++++++++++
>>> target-lm32/cpu.h | 14 ++++++++++++++
>>> target-m68k/cpu.h | 14 ++++++++++++++
>>> target-microblaze/cpu.h | 14 ++++++++++++++
>>> target-mips/cpu.h | 14 ++++++++++++++
>>> target-moxie/cpu.h | 14 ++++++++++++++
>>> target-openrisc/cpu.h | 14 ++++++++++++++
>>> target-ppc/cpu.h | 14 ++++++++++++++
>>> target-s390x/cpu.h | 14 ++++++++++++++
>>> target-sh4/cpu.h | 14 ++++++++++++++
>>> target-sparc/cpu.h | 14 ++++++++++++++
>>> target-sparc/translate.c | 1 +
>>> target-tilegx/cpu.h | 14 ++++++++++++++
>>> target-tricore/cpu.h | 14 ++++++++++++++
>>> target-unicore32/cpu.h | 14 ++++++++++++++
>>> target-xtensa/cpu.h | 14 ++++++++++++++
>>> 20 files changed, 266 insertions(+)
>>>
>>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
>>> index db79ab65cebe..61cc3a1fb8f7 100644
>>> --- a/include/exec/exec-all.h
>>> +++ b/include/exec/exec-all.h
>>> @@ -256,6 +256,16 @@ void tb_free(TranslationBlock *tb);
>>> void tb_flush(CPUState *cpu);
>>> void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
>>>
>>> +static inline void tb_mark_invalid(TranslationBlock *tb)
>>> +{
>>> + cpu_get_invalid_tb_cpu_state(&tb->pc, &tb->cs_base, &tb->flags);
>> Hmmm are we getting something here? Maybe cpu_tb_invalidate_cpu_state?
>
> Just to be similar to cpu_get_tb_cpu_state().
I guess if you squint just right. I'll drop the objection.
>
>>
>>> +}
>>> +
>>> +static inline bool tb_is_invalid(TranslationBlock *tb)
>>> +{
>>> + return cpu_tb_cpu_state_is_invalidated(tb->pc, tb->cs_base, tb->flags);
>>> +}
>> Also why are we passing three pointers to parts of TranslationBlock? Why
>> not just pass tb directly and be done with it?
>
> I'm not sure we want to include exec/exec-all.h in target-*/cpu.h
Fair enough.
>
>>
>> I'm sure the compiler does something sensible but seems overly verbose
>> to me.
>>
>>> +
>>> #if defined(USE_DIRECT_JUMP)
>>>
>>> #if defined(CONFIG_TCG_INTERPRETER)
> (snip)
>> Otherwise:
>>
>> Reviewed-by: Alex Bennée <address@hidden>
>
> Kind regards,
> Sergey
--
Alex Bennée