[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code
From: |
Peter Maydell |
Subject: |
Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code |
Date: |
Thu, 15 Apr 2021 14:37:06 +0100 |
On Thu, 15 Apr 2021 at 14:18, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 18 Feb 2021 at 09:47, Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> > There is no real need to use CF_NOCACHE here. As long as the TB isn't
> > linked to other TBs or included in the QHT or jump cache then it will
> > only get executed once.
> >
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > Message-Id: <20210213130325.14781-19-alex.bennee@linaro.org>
>
> Hi; I've just noticed that this commit seems to break the case of:
> * execution of code not from a RAM block
> * when icount is enabled
> * and an instruction is an IO insn that triggers io-recompile
>
> because:
>
> > @@ -2097,6 +2086,17 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> > tb_reset_jump(tb, 1);
> > }
> >
> > + /*
> > + * If the TB is not associated with a physical RAM page then
> > + * it must be a temporary one-insn TB, and we have nothing to do
> > + * except fill in the page_addr[] fields. Return early before
> > + * attempting to link to other TBs or add to the lookup table.
> > + */
> > + if (phys_pc == -1) {
> > + tb->page_addr[0] = tb->page_addr[1] = -1;
> > + return tb;
> > + }
>
> we used to fall through here, which meant we called
> tcg_tb_insert(tb). No we no longer do. That's bad, because
> cpu_io_recompile() does:
>
> tb = tcg_tb_lookup(retaddr);
> if (!tb) {
> cpu_abort(cpu, "cpu_io_recompile: could not find TB for pc=%p",
> (void *)retaddr);
> }
>
> and since it can no longer find the TB, QEMU aborts.
Adding the tcg_tb_insert() call to the early exit path:
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ba6ab09790e..6014285e4dc 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2081,6 +2081,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
*/
if (phys_pc == -1) {
tb->page_addr[0] = tb->page_addr[1] = -1;
+ tcg_tb_insert(tb);
return tb;
}
seems to fix my test case, but I don't know enough about the new
design here to know if that has undesirable side effects.
-- PMM
- Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Peter Maydell, 2021/04/15
- Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code,
Peter Maydell <=
- Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Alex Bennée, 2021/04/15
- Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Peter Maydell, 2021/04/15
- Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Philippe Mathieu-Daudé, 2021/04/15
- Re: [EXTERNAL] Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Cédric Le Goater, 2021/04/15
- Re: [EXTERNAL] Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Peter Maydell, 2021/04/15
- Re: [EXTERNAL] Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Cédric Le Goater, 2021/04/16
- Re: [EXTERNAL] Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Alex Bennée, 2021/04/16
- Re: [EXTERNAL] Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code, Cédric Le Goater, 2021/04/16