| Message ID | 20210415162454.22056-1-alex.bennee@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] accel/tcg: avoid re-translating one-shot instructions | expand |
On Thu, 15 Apr 2021 at 17:25, Alex Bennée <alex.bennee@linaro.org> wrote: > > By definition a single instruction is capable of being an IO > instruction. This avoids a problem of triggering a cpu_io_recompile on > a non-recorded translation which then fails because it expects > tcg_tb_lookup() to succeed unconditionally. The normal use case > requires a TB to be able to resolve machine state. > > The other users of tcg_tb_lookup() are able to tolerate a missing TB > if the machine state has been resolved by other means - which in the > single-shot case is always true because machine state is synced at the > start of a block. > > Reported-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > accel/tcg/translate-all.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > index ba6ab09790..b12d0898d0 100644 > --- a/accel/tcg/translate-all.c > +++ b/accel/tcg/translate-all.c > @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, > > if (phys_pc == -1) { > /* Generate a one-shot TB with 1 insn in it */ > - cflags = (cflags & ~CF_COUNT_MASK) | 1; > + cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1; > } > > max_insns = cflags & CF_COUNT_MASK; > -- Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 4/15/21 9:24 AM, Alex Bennée wrote: > By definition a single instruction is capable of being an IO > instruction. This avoids a problem of triggering a cpu_io_recompile on > a non-recorded translation which then fails because it expects > tcg_tb_lookup() to succeed unconditionally. The normal use case > requires a TB to be able to resolve machine state. > > The other users of tcg_tb_lookup() are able to tolerate a missing TB > if the machine state has been resolved by other means - which in the > single-shot case is always true because machine state is synced at the > start of a block. > > Reported-by: Peter Maydell<peter.maydell@linaro.org> > Signed-off-by: Alex Bennée<alex.bennee@linaro.org> > --- > accel/tcg/translate-all.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell <peter.maydell@linaro.org> writes: > On Thu, 15 Apr 2021 at 17:25, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> By definition a single instruction is capable of being an IO >> instruction. This avoids a problem of triggering a cpu_io_recompile on >> a non-recorded translation which then fails because it expects >> tcg_tb_lookup() to succeed unconditionally. The normal use case >> requires a TB to be able to resolve machine state. >> >> The other users of tcg_tb_lookup() are able to tolerate a missing TB >> if the machine state has been resolved by other means - which in the >> single-shot case is always true because machine state is synced at the >> start of a block. >> >> Reported-by: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> accel/tcg/translate-all.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c >> index ba6ab09790..b12d0898d0 100644 >> --- a/accel/tcg/translate-all.c >> +++ b/accel/tcg/translate-all.c >> @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, >> >> if (phys_pc == -1) { >> /* Generate a one-shot TB with 1 insn in it */ >> - cflags = (cflags & ~CF_COUNT_MASK) | 1; >> + cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1; >> } >> >> max_insns = cflags & CF_COUNT_MASK; >> -- > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Are you going to apply this directly or do you want it through a tree? > > thanks > -- PMM
On Thu, 15 Apr 2021 at 19:13, Alex Bennée <alex.bennee@linaro.org> wrote: > > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Thu, 15 Apr 2021 at 17:25, Alex Bennée <alex.bennee@linaro.org> wrote: > >> > >> By definition a single instruction is capable of being an IO > >> instruction. This avoids a problem of triggering a cpu_io_recompile on > >> a non-recorded translation which then fails because it expects > >> tcg_tb_lookup() to succeed unconditionally. The normal use case > >> requires a TB to be able to resolve machine state. > >> > >> The other users of tcg_tb_lookup() are able to tolerate a missing TB > >> if the machine state has been resolved by other means - which in the > >> single-shot case is always true because machine state is synced at the > >> start of a block. > >> > >> Reported-by: Peter Maydell <peter.maydell@linaro.org> > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > >> --- > >> accel/tcg/translate-all.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > >> index ba6ab09790..b12d0898d0 100644 > >> --- a/accel/tcg/translate-all.c > >> +++ b/accel/tcg/translate-all.c > >> @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, > >> > >> if (phys_pc == -1) { > >> /* Generate a one-shot TB with 1 insn in it */ > >> - cflags = (cflags & ~CF_COUNT_MASK) | 1; > >> + cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1; > >> } > >> > >> max_insns = cflags & CF_COUNT_MASK; > >> -- > > > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > Are you going to apply this directly or do you want it through a tree? For the moment I've just added it to the list of "not fixed in 6.0 but not quite meriting an rc4" items on the Planning page. If we need an rc4 I can apply it directly. (Though if you did whatever testing you'd do on a pullreq that's beyond just "run it through gitlab" that would be useful I think.) thanks -- PMM
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index ba6ab09790..b12d0898d0 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, if (phys_pc == -1) { /* Generate a one-shot TB with 1 insn in it */ - cflags = (cflags & ~CF_COUNT_MASK) | 1; + cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1; } max_insns = cflags & CF_COUNT_MASK;
By definition a single instruction is capable of being an IO instruction. This avoids a problem of triggering a cpu_io_recompile on a non-recorded translation which then fails because it expects tcg_tb_lookup() to succeed unconditionally. The normal use case requires a TB to be able to resolve machine state. The other users of tcg_tb_lookup() are able to tolerate a missing TB if the machine state has been resolved by other means - which in the single-shot case is always true because machine state is synced at the start of a block. Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- accel/tcg/translate-all.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)