[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/8] tcg: Use uintptr_t type for jmp_list_{next|
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH 2/8] tcg: Use uintptr_t type for jmp_list_{next|first} fields of TB |
Date: |
Thu, 24 Mar 2016 14:58:56 +0000 |
User-agent: |
mu4e 0.9.17; emacs 25.0.92.7 |
address@hidden writes:
> From: Sergey Fedorov <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Sergey Fedorov <address@hidden>
> ---
> include/exec/exec-all.h | 12 +++++++-----
> translate-all.c | 37 +++++++++++++++++++------------------
> 2 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index cc3d2ca25917..cd96219a89e7 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -275,14 +275,15 @@ 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, and the two least significant bits of them
> + * are used to encode which data field holds a pointer to the next 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.
> */
Ahh I see you anticipate my previous confusion. Does this mean each time
a jump is resolved for a particular chain the next tb could be in a
different entry in the next TB?
> - 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"
> @@ -396,7 +397,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 31cdf8ae217e..7c008927e3f3 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -926,17 +926,16 @@ 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;
> 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);
> + n1 = *ptb & 3;
> + tb1 = (TranslationBlock *)(*ptb & ~3);
> if (n1 == n && tb1 == tb) {
> break;
> }
> @@ -949,7 +948,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;
> }
> }
>
> @@ -968,7 +967,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);
> @@ -1004,19 +1003,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++;
> }
> @@ -1489,9 +1489,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
- [Qemu-devel] [PATCH 0/8] tcg: Direct block chaining clean-up, sergey . fedorov, 2016/03/24
- [Qemu-devel] [PATCH 1/8] tcg: Clean up direct block chaining data fields, sergey . fedorov, 2016/03/24
- Re: [Qemu-devel] [PATCH 1/8] tcg: Clean up direct block chaining data fields, Alex Bennée, 2016/03/24
- Re: [Qemu-devel] [PATCH 1/8] tcg: Clean up direct block chaining data fields, Sergey Fedorov, 2016/03/24
- Re: [Qemu-devel] [PATCH 1/8] tcg: Clean up direct block chaining data fields, Alex Bennée, 2016/03/24
- Re: [Qemu-devel] [PATCH 1/8] tcg: Clean up direct block chaining data fields, Sergey Fedorov, 2016/03/24
- Re: [Qemu-devel] [PATCH 1/8] tcg: Clean up direct block chaining data fields, Paolo Bonzini, 2016/03/24
- Re: [Qemu-devel] [PATCH 1/8] tcg: Clean up direct block chaining data fields, Alex Bennée, 2016/03/24
[Qemu-devel] [PATCH 2/8] tcg: Use uintptr_t type for jmp_list_{next|first} fields of TB, sergey . fedorov, 2016/03/24
[Qemu-devel] [PATCH 5/8] tcg: Clarify "thread safaty" check in tb_add_jump(), sergey . fedorov, 2016/03/24
[Qemu-devel] [PATCH 3/8] tcg: Rearrange tb_link_page() to avoid forward declaration, sergey . fedorov, 2016/03/24
[Qemu-devel] [PATCH 6/8] tcg: Rename tb_jmp_remove() to tb_remove_from_jmp_list(), sergey . fedorov, 2016/03/24