Re: [PULL 24/27] accel/tcg: Move breakpoint recognition outside translation

[Richard Henderson]

On 8/17/21 3:33 AM, Peter Maydell wrote:
> On Wed, 21 Jul 2021 at 21:00, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > Trigger breakpoints before beginning translation of a TB
> > that would begin with a BP.  Thus we never generate code
> > for the BP at all.
>
> I happened to notice in the Arm ARM today a corner case that this
> implementation approach I think gets wrong: the priority ordering
> of exceptions is supposed to be (among others)
>   * (architectural) software step
>   * instruction abort
>   * (architectural) breakpoints
>
> I think that doing the bp check here means it is incorrectly
> hoisted up the priority order above both swstep and insn
> abort.

Hmm, you're correct that we get this wrong.

> We probably didn't do these in the right priority
> order before this series, though, and I dunno whether
> we get the insn-abort vs swstep ordering right either...

And you're correct that we got it wrong beforehand.  The reorg did not alter the 
recognized ordering of the exceptions.

I'm a bit surprised that insn-abort comes higher than breakpoint.  Fixing this would mean 
performing the insn decode and only then recognizing the breakpoint.  One of the 
intermediate versions of the patch set would have allowed this sort of thing, but I didn't 
realize it was necessary.  And it would be a huge job to alter all of the trans_* functions.

Fixing the order of swstep and bp can be done via the arm_debug_check_breakpoint hook. 
Just return false if swstep is enabled.


r~