[PATCH 0/6] target/microblaze: Use tcg_gen_lookup_and_goto_ptr

[Richard Henderson]

Based-on: <20200831160601.833692-1-richard.henderson@linaro.org>
("[PULL 00/76] target/microblaze improvements")

Hello again, Edgar.

I had dropped the tcg_gen_lookup_and_goto_ptr patch from the
previous omnibus patch set, as you had reported lockups.

I have identified, by inspection, two cases in which we failed
to return to the main loop even though we should have:

(1) Return-from-exception type instructions.

I had missed these before because they hadn't set cpustate_changed.
This still worked fine because they are all indirect branches, and
had exited immediately.

Fixed by distinguishing these cases from normal indirect branches
before we start using lookup_and_goto_ptr.

(2) MTS in a branch delay slot.

We did not check dc->cpustate_changed before setting
dc->base.is_jmp to DISAS_JUMP, which lost the fact that we
need to return to the main loop.

This mostly works fine without lookup_and_goto_ptr, because
we either (a) finished an indirect branch and returned to the
main loop anyway or (b) we'd return to the main loop via some
subsequent indirect branch, which would happen "soon enough".

We should have been able to see soft-lockup with the existing
code in the case of a cpustate_changed in the delay slot of
a loop of direct branches that all use goto_tb.  E.g.

        brid    0
         msrset MSR_IE

I.e. an immediate branch back to the same branch insn,
re-enabling interrupts in the delay slot.  Probably not
something that shows up in the wild.

----

Follow-up question: The manual says that several classes of
instructions are invalid in a branch delay slot, but does
not say what action is taken, if any.

Some of these invalid cases could leave qemu in an inconsistent
state.  Would it be legal for us to diagnose these cases with
trap_illegal?  If not, what *should* we be doing?  We could also
LOG_GUEST_ERROR for these either way.

I've added some TODO comments in these patches that are relevant.

Thanks,


r~


Richard Henderson (6):
  target/microblaze: Rename DISAS_UPDATE to DISAS_EXIT
  target/microblaze: Introduce DISAS_EXIT_NEXT, DISAS_EXIT_JUMP
  target/microblaze: Replace cpustate_changed with DISAS_EXIT_NEXT
  target/microblaze: Handle DISAS_EXIT_NEXT in delay slot
  target/microblaze: Force rtid, rted, rtbd to exit
  target/microblaze: Use tcg_gen_lookup_and_goto_ptr

 target/microblaze/translate.c | 128 ++++++++++++++++++++++------------
 1 file changed, 82 insertions(+), 46 deletions(-)

-- 
2.25.1