Sergey Fedorov <sergey.fedo...@linaro.org> writes:
> From: Sergey Fedorov <serge.f...@gmail.com>
>
> These fields do not contain pure pointers to a TranslationBlock
> structure. So uintptr_t is the most appropriate type for them.
> Also put an assert to assure that the two least significant bits of the
> pointer are zero before assigning it to jmp_list_first.
>
> Signed-off-by: Sergey Fedorov <serge.f...@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedo...@linaro.org>
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
> ---
>
> Changes in v2:
> * Eliminated duplicate dereference of 'ptb' in tb_jmp_remove()
>
> include/exec/exec-all.h | 13 ++++++++-----
> translate-all.c | 38 ++++++++++++++++++++------------------
> 2 files changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 38a149cc5e0c..b055716ed690 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -275,14 +275,16 @@ struct TranslationBlock {
> * jmp_list_first points to the first TB jumping to this one.
> * jmp_list_next is used to point to the next TB in a list.
> * Since each TB can have two jumps, it can participate in two lists.
> - * The two least significant bits of a pointer are used to choose which
> - * data field holds a pointer to the next TB:
> + * jmp_list_first and jmp_list_next are 4-byte aligned pointers to a
> + * TranslationBlock structure, but the two least significant bits of
> + * them are used to encode which data field of the pointed TB should
> + * be used to traverse the list further from that TB:
> * 0 => jmp_list_next[0], 1 => jmp_list_next[1], 2 => jmp_list_first.
> * In other words, 0/1 tells which jump is used in the pointed TB,
> * and 2 means that this is a pointer back to the target TB of this list.
> */
> - struct TranslationBlock *jmp_list_next[2];
> - struct TranslationBlock *jmp_list_first;
> + uintptr_t jmp_list_next[2];
> + uintptr_t jmp_list_first;
> };
>
> #include "qemu/thread.h"
> @@ -401,7 +403,8 @@ static inline void tb_add_jump(TranslationBlock *tb, int
> n,
>
> /* add in TB jmp circular list */
> tb->jmp_list_next[n] = tb_next->jmp_list_first;
> - tb_next->jmp_list_first = (TranslationBlock *)((uintptr_t)tb | n);
> + assert(((uintptr_t)tb & 3) == 0);
> + tb_next->jmp_list_first = (uintptr_t)tb | n;
> }
> }
>
> diff --git a/translate-all.c b/translate-all.c
> index 33ca06c663d4..ba71ff73f55f 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -927,17 +927,17 @@ static inline void tb_page_remove(TranslationBlock
> **ptb, TranslationBlock *tb)
>
> static inline void tb_jmp_remove(TranslationBlock *tb, int n)
> {
> - TranslationBlock *tb1, **ptb;
> + TranslationBlock *tb1;
> + uintptr_t *ptb, ntb;
> unsigned int n1;
>
> ptb = &tb->jmp_list_next[n];
> - tb1 = *ptb;
> - if (tb1) {
> + if (*ptb) {
> /* find tb(n) in circular list */
> for (;;) {
> - tb1 = *ptb;
> - n1 = (uintptr_t)tb1 & 3;
> - tb1 = (TranslationBlock *)((uintptr_t)tb1 & ~3);
> + ntb = *ptb;
> + n1 = ntb & 3;
> + tb1 = (TranslationBlock *)(ntb & ~3);
> if (n1 == n && tb1 == tb) {
> break;
> }
> @@ -950,7 +950,7 @@ static inline void tb_jmp_remove(TranslationBlock *tb,
> int n)
> /* now we can suppress tb(n) from the list */
> *ptb = tb->jmp_list_next[n];
>
> - tb->jmp_list_next[n] = NULL;
> + tb->jmp_list_next[n] = (uintptr_t)NULL;
> }
> }
>
> @@ -969,7 +969,7 @@ void tb_phys_invalidate(TranslationBlock *tb,
> tb_page_addr_t page_addr)
> PageDesc *p;
> unsigned int h, n1;
> tb_page_addr_t phys_pc;
> - TranslationBlock *tb1, *tb2;
> + uintptr_t tb1, tb2;
>
> /* remove the TB from the hash list */
> phys_pc = tb->page_addr[0] + (tb->pc & ~TARGET_PAGE_MASK);
> @@ -1005,19 +1005,20 @@ void tb_phys_invalidate(TranslationBlock *tb,
> tb_page_addr_t page_addr)
> /* suppress any remaining jumps to this TB */
> tb1 = tb->jmp_list_first;
> for (;;) {
> - n1 = (uintptr_t)tb1 & 3;
> + TranslationBlock *tmp_tb;
> + n1 = tb1 & 3;
> if (n1 == 2) {
> break;
> }
> - tb1 = (TranslationBlock *)((uintptr_t)tb1 & ~3);
> - tb2 = tb1->jmp_list_next[n1];
> - tb_reset_jump(tb1, n1);
> - tb1->jmp_list_next[n1] = NULL;
> + tmp_tb = (TranslationBlock *)(tb1 & ~3);
> + tb2 = tmp_tb->jmp_list_next[n1];
> + tb_reset_jump(tmp_tb, n1);
> + tmp_tb->jmp_list_next[n1] = (uintptr_t)NULL;
> tb1 = tb2;
> }
>
> - /* fail safe */
> - tb->jmp_list_first = (TranslationBlock *)((uintptr_t)tb | 2);
> + assert(((uintptr_t)tb & 3) == 0);
> + tb->jmp_list_first = (uintptr_t)tb | 2; /* fail safe */
>
> tcg_ctx.tb_ctx.tb_phys_invalidate_count++;
> }
> @@ -1491,9 +1492,10 @@ static void tb_link_page(TranslationBlock *tb,
> tb_page_addr_t phys_pc,
> tb->page_addr[1] = -1;
> }
>
> - tb->jmp_list_first = (TranslationBlock *)((uintptr_t)tb | 2);
> - tb->jmp_list_next[0] = NULL;
> - tb->jmp_list_next[1] = NULL;
> + assert(((uintptr_t)tb & 3) == 0);
> + tb->jmp_list_first = (uintptr_t)tb | 2;
> + tb->jmp_list_next[0] = (uintptr_t)NULL;
> + tb->jmp_list_next[1] = (uintptr_t)NULL;
>
> /* init original jump addresses */
> if (tb->jmp_reset_offset[0] != TB_JMP_RESET_OFFSET_INVALID) {
--
Alex Bennée
