On 17/03/16 23:45, Sergey Fedorov
wrote:
On
17/03/16 22:31, Paolo Bonzini wrote:
On 17/03/2016 18:57, Richard Henderson wrote:
@@ -951,18
+959,10 @@ static inline void tb_jmp_remove(TranslationBlock
*tb, int n)
}
/* now we can suppress tb(n) from the list */
*ptb = tb->jmp_next[n];
-
- tb->jmp_next[n] = NULL;
+ tb_reset_jump(tb, n);
What's the motivation here? This implies an extra cache
flush.
Where were we resetting the jump previously? Or is this a bug
in that we *weren't* resetting the jump
previously?
Indeed I think this patch can be removed if it has a performance
effect
on machines that require icache invalidation. If it doesn't, it
would
be just a small code simplification.
In fact, tb_jmp_remove() is only supposed to remove the TB from a
list of all TB's jumping to the same TB which is n-th jump
destination of the given TB. This function is only called in
tb_phys_invalidate() for the TB being invalidated. Thus we don't
have to patch that TB anymore. We don't even have to do
"tb->jmp_next[n] = NULL" here.
I'll drop the patch from the series in v2 then.
Kind regards,
Sergey
|