| Message ID | 20230331150609.114401-7-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: Fix pointer mask related support | expand |
On 3/31/23 08:06, Weiwei Li wrote: > A corner case is triggered when tb block with first_pc = 0x80000008 > and first_pc = 0x800000200 has the same jump cache hash, and share > the same tb entry with the same tb information except PC. > The executed sequence is as follows: > tb(0x80000008) -> tb(0x80000008)-> tb(0x800000200) -> tb(0x80000008) > > 1. At the first time tb for 0x80000008 is loaded, tb in jmp_cache is > filled, however pc is not updated. > 2. At the second time tb for 0x80000008 is looked up in tb_lookup(), > pc in jmp cache is set to 0x80000008. > 3. when tb for 0x800000200 is loaded, tb for jmp cache is updated to > this block, however pc is not updated, and remains to be 0x80000008. > 4. Finally at the last time tb for 0x80000008 is looked up, tb for > 0x800000200 is mismatched. > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > --- > accel/tcg/cpu-exec.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index c815f2dbfd..faff413f42 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -983,6 +983,9 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc) > h = tb_jmp_cache_hash_func(pc); > /* Use the pc value already stored in tb->pc. */ > qatomic_set(&cpu->tb_jmp_cache->array[h].tb, tb); > + if (cflags & CF_PCREL) { > + qatomic_set(&cpu->tb_jmp_cache->array[h].pc, pc); > + } Good catch on the bug, but incorrect fix. Need if (cflags & CF_PCREL) { qatomic_set(&cpu->tb_jmp_cache->array[h].pc, pc); qatomic_store_release(&cpu->tb_jmp_cache->array[h].tb, tb); } else { /* Use the pc value already stored in tb->pc. */ qatomic_set(&cpu->tb_jmp_cache->array[h].tb, tb); } r~
On 3/31/23 18:26, Richard Henderson wrote: > On 3/31/23 08:06, Weiwei Li wrote: >> A corner case is triggered when tb block with first_pc = 0x80000008 >> and first_pc = 0x800000200 has the same jump cache hash, and share >> the same tb entry with the same tb information except PC. >> The executed sequence is as follows: >> tb(0x80000008) -> tb(0x80000008)-> tb(0x800000200) -> tb(0x80000008) >> >> 1. At the first time tb for 0x80000008 is loaded, tb in jmp_cache is >> filled, however pc is not updated. >> 2. At the second time tb for 0x80000008 is looked up in tb_lookup(), >> pc in jmp cache is set to 0x80000008. >> 3. when tb for 0x800000200 is loaded, tb for jmp cache is updated to >> this block, however pc is not updated, and remains to be 0x80000008. >> 4. Finally at the last time tb for 0x80000008 is looked up, tb for >> 0x800000200 is mismatched. >> >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> >> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> >> --- >> accel/tcg/cpu-exec.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >> index c815f2dbfd..faff413f42 100644 >> --- a/accel/tcg/cpu-exec.c >> +++ b/accel/tcg/cpu-exec.c >> @@ -983,6 +983,9 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc) >> h = tb_jmp_cache_hash_func(pc); >> /* Use the pc value already stored in tb->pc. */ >> qatomic_set(&cpu->tb_jmp_cache->array[h].tb, tb); >> + if (cflags & CF_PCREL) { >> + qatomic_set(&cpu->tb_jmp_cache->array[h].pc, pc); >> + } > > Good catch on the bug, but incorrect fix. Need > > if (cflags & CF_PCREL) { > qatomic_set(&cpu->tb_jmp_cache->array[h].pc, pc); > qatomic_store_release(&cpu->tb_jmp_cache->array[h].tb, tb); > } else { > /* Use the pc value already stored in tb->pc. */ > qatomic_set(&cpu->tb_jmp_cache->array[h].tb, tb); > } Queuing the fix to tcg-next. r~
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index c815f2dbfd..faff413f42 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -983,6 +983,9 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc) h = tb_jmp_cache_hash_func(pc); /* Use the pc value already stored in tb->pc. */ qatomic_set(&cpu->tb_jmp_cache->array[h].tb, tb); + if (cflags & CF_PCREL) { + qatomic_set(&cpu->tb_jmp_cache->array[h].pc, pc); + } } #ifndef CONFIG_USER_ONLY