* [PATCH] accel/tcg: nochain should disable goto_ptr @ 2024-05-31 10:17 NiuGenen 2024-05-31 17:32 ` Richard Henderson 0 siblings, 1 reply; 5+ messages in thread From: NiuGenen @ 2024-05-31 10:17 UTC (permalink / raw) To: qemu-devel; +Cc: NiuGenen Signed-off-by: NiuGenen <niugen@loongson.cn> --- accel/tcg/cpu-exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 2972f75b96..084fa645c7 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -173,7 +173,7 @@ uint32_t curr_cflags(CPUState *cpu) } else if (qatomic_read(&one_insn_per_tb)) { cflags |= CF_NO_GOTO_TB | 1; } else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { - cflags |= CF_NO_GOTO_TB; + cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR; } return cflags; -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] accel/tcg: nochain should disable goto_ptr 2024-05-31 10:17 [PATCH] accel/tcg: nochain should disable goto_ptr NiuGenen @ 2024-05-31 17:32 ` Richard Henderson 2024-06-01 5:57 ` niugen 0 siblings, 1 reply; 5+ messages in thread From: Richard Henderson @ 2024-05-31 17:32 UTC (permalink / raw) To: NiuGenen, qemu-devel On 5/31/24 03:17, NiuGenen wrote: > Signed-off-by: NiuGenen <niugen@loongson.cn> > --- > accel/tcg/cpu-exec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index 2972f75b96..084fa645c7 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -173,7 +173,7 @@ uint32_t curr_cflags(CPUState *cpu) > } else if (qatomic_read(&one_insn_per_tb)) { > cflags |= CF_NO_GOTO_TB | 1; > } else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { > - cflags |= CF_NO_GOTO_TB; > + cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR; > } > > return cflags; Why? The original intent of nochain was so that -d exec would log all blocks, which requires excluding goto_tb. There is exec logging in helper_lookup_goto_ptr, so there is no need to avoid goto_ptr. You must provide a rationale, at minimum. r~ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] accel/tcg: nochain should disable goto_ptr 2024-05-31 17:32 ` Richard Henderson @ 2024-06-01 5:57 ` niugen 2024-06-01 13:36 ` Richard Henderson 0 siblings, 1 reply; 5+ messages in thread From: niugen @ 2024-06-01 5:57 UTC (permalink / raw) To: Richard Henderson, qemu-devel on 2024/6/1 01:32, Richard Henderson wrote: > On 5/31/24 03:17, NiuGenen wrote: >> Signed-off-by: NiuGenen <niugen@loongson.cn> >> --- >> accel/tcg/cpu-exec.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >> index 2972f75b96..084fa645c7 100644 >> --- a/accel/tcg/cpu-exec.c >> +++ b/accel/tcg/cpu-exec.c >> @@ -173,7 +173,7 @@ uint32_t curr_cflags(CPUState *cpu) >> } else if (qatomic_read(&one_insn_per_tb)) { >> cflags |= CF_NO_GOTO_TB | 1; >> } else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { >> - cflags |= CF_NO_GOTO_TB; >> + cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR; >> } >> return cflags; > > Why? > > The original intent of nochain was so that -d exec would log all > blocks, which requires excluding goto_tb. There is exec logging in > helper_lookup_goto_ptr, so there is no need to avoid goto_ptr. > > You must provide a rationale, at minimum. > > > r~ Sorry, my mistake. I thought nochain will disable all kinds of branches, including direct branch and indirect branch, but I found that indirect branch still call helper_lookup_tb_ptr to continue executing TB instead of epilogue-tblookup-prologue. Maybe the exec logging can be removed from helper_lookup_tb_ptr and nochain can disable all the chaining of TB? Thanks for your patience. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] accel/tcg: nochain should disable goto_ptr 2024-06-01 5:57 ` niugen @ 2024-06-01 13:36 ` Richard Henderson 2024-06-03 1:51 ` niugen 0 siblings, 1 reply; 5+ messages in thread From: Richard Henderson @ 2024-06-01 13:36 UTC (permalink / raw) To: niugen, qemu-devel On 6/1/24 00:57, niugen wrote: > on 2024/6/1 01:32, Richard Henderson wrote: >> On 5/31/24 03:17, NiuGenen wrote: >>> Signed-off-by: NiuGenen <niugen@loongson.cn> >>> --- >>> accel/tcg/cpu-exec.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >>> index 2972f75b96..084fa645c7 100644 >>> --- a/accel/tcg/cpu-exec.c >>> +++ b/accel/tcg/cpu-exec.c >>> @@ -173,7 +173,7 @@ uint32_t curr_cflags(CPUState *cpu) >>> } else if (qatomic_read(&one_insn_per_tb)) { >>> cflags |= CF_NO_GOTO_TB | 1; >>> } else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { >>> - cflags |= CF_NO_GOTO_TB; >>> + cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR; >>> } >>> return cflags; >> >> Why? >> >> The original intent of nochain was so that -d exec would log all blocks, which requires >> excluding goto_tb. There is exec logging in helper_lookup_goto_ptr, so there is no need >> to avoid goto_ptr. >> >> You must provide a rationale, at minimum. >> >> >> r~ > > > Sorry, my mistake. I thought nochain will disable all kinds of branches, including direct > branch and indirect branch, but I found that indirect branch still call > helper_lookup_tb_ptr to continue executing TB instead of epilogue-tblookup-prologue. > > Maybe the exec logging can be removed from helper_lookup_tb_ptr and nochain can disable > all the chaining of TB? Why? You still haven't provided a rationale. r~ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] accel/tcg: nochain should disable goto_ptr 2024-06-01 13:36 ` Richard Henderson @ 2024-06-03 1:51 ` niugen 0 siblings, 0 replies; 5+ messages in thread From: niugen @ 2024-06-03 1:51 UTC (permalink / raw) To: Richard Henderson, qemu-devel On 2024/6/1 21:36, Richard Henderson wrote: > On 6/1/24 00:57, niugen wrote: >> on 2024/6/1 01:32, Richard Henderson wrote: >>> On 5/31/24 03:17, NiuGenen wrote: >>>> Signed-off-by: NiuGenen <niugen@loongson.cn> >>>> --- >>>> accel/tcg/cpu-exec.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >>>> index 2972f75b96..084fa645c7 100644 >>>> --- a/accel/tcg/cpu-exec.c >>>> +++ b/accel/tcg/cpu-exec.c >>>> @@ -173,7 +173,7 @@ uint32_t curr_cflags(CPUState *cpu) >>>> } else if (qatomic_read(&one_insn_per_tb)) { >>>> cflags |= CF_NO_GOTO_TB | 1; >>>> } else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { >>>> - cflags |= CF_NO_GOTO_TB; >>>> + cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR; >>>> } >>>> return cflags; >>> >>> Why? >>> >>> The original intent of nochain was so that -d exec would log all >>> blocks, which requires excluding goto_tb. There is exec logging in >>> helper_lookup_goto_ptr, so there is no need to avoid goto_ptr. >>> >>> You must provide a rationale, at minimum. >>> >>> >>> r~ >> >> >> Sorry, my mistake. I thought nochain will disable all kinds of >> branches, including direct branch and indirect branch, but I found >> that indirect branch still call helper_lookup_tb_ptr to continue >> executing TB instead of epilogue-tblookup-prologue. >> >> Maybe the exec logging can be removed from helper_lookup_tb_ptr and >> nochain can disable all the chaining of TB? > > Why? You still haven't provided a rationale. > > > r~ Currently, nochain is actually no-direct-chaining(i.e. no-goto-tb). And it works fine with -d exec because of the helper_lookup_tb_ptr generates the exec logging too. Other optimizations for indirect branch (eg. shadow-stack, inlined CPUJumpCache lookup, maybe developed in the future) that use goto_ptr will need to generate the exec logging too. It makes the codes more clear if nochain is no-any-chaining. Any logging or other mechanism that needs to operate per TB's execution can be done in cpu_tb_exec only. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-03 2:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-05-31 10:17 [PATCH] accel/tcg: nochain should disable goto_ptr NiuGenen 2024-05-31 17:32 ` Richard Henderson 2024-06-01 5:57 ` niugen 2024-06-01 13:36 ` Richard Henderson 2024-06-03 1:51 ` niugen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).