Re: [Qemu-devel] [PATCH 01/12] Refactor translation block CPU state handling

[Anthony Liguori <anthony@codemonkey.ws>]

Jan Kiszka wrote:
> This patch refactors the way the CPU state is handled that is associated
> with a TB. The basic motivation is to move more arch specific code out
> of generic files. Specifically the long #ifdef clutter in tb_find_fast()
> has to be overcome in order to avoid duplicating it for the gdb
> watchpoint fixes (patch "Restore pc on watchpoint hits").
>
> The approach taken here is to encapsulate the relevant CPU state in a
> new structure called TBCPUState which is kept inside TranslationBlock
> but also passed around when setting up new TBs. To fill a TBCPUState
> based on the current CPUState, each arch has to provide
> cpu_get_tb_cpu_state(CPUState *, TBCPUState *).
>
> At this chance, the patch also converts the not really beautiful macro
> CPU_PC_FROM_TB into a clean static inline function.
>   

There are really three patches in one here.  The first patch converts 
CPU_PC_FROM_TB to a function.  The second eliminates the #ifdef mess in 
tb_find_fast by introducing cpu_get_tb_cpu_state(CPUState *, 
TranslationBlock *), and the third moves the CPU specific state in 
Translation block to a separate structure and changes the signature of 
cpu_get_tb_cpu_state() to take the TBCPUState structure.

Can you split up this patch along these lines?  All of these changes are 
mechanical and they are easier to review/bisect as separate patches.

I'm not 100% convinced that the third patch is really that valuable.  
Can you explain if there's any benefit to doing this other than readability?

Regards,

Anthony Liguori