On 13 November 2015 at 21:13, Sergey Fedorov <serge.f...@gmail.com> wrote:
> On 10.11.2015 15:15, Peter Maydell wrote:
>> So the way the 32-bit code works for singlestep is complicated
>> because of the need to handle the conditional instructions,
>> which means you get a lot more cases like "this is a conditional
>> SWI" that need to be handled. A quick summary of some of the
>> possible cases:
>>
>> * unconditional normal instruction:
>> -- need to write the PC and condexec bits back to the CPU state
>> -- then take a singlestep insn (either the architectural one
>> or the EXCP_DEBUG one depending on which sort of step we are doing)
>> * unconditional exception-generating instruction
>> -- for architectural step of SWI/HVC/SMC we need to advance the
>> singlestep state machine so that they behave correctly
>> -- generate the relevant exception and then no point writing the
>> code to take EXCP_DEBUG &c because we won't get to it
>> * conditional instruction (including cond. branches):
>> -- earlier code has already written back the PC for the
>> "condition passed" case
>> -- write out the code which takes the singlestep exception for
>> the "condition passed" case
>> -- then do gen_set_label(dc->condlabel)
>> -- then the code to take the single step exception after
>> executing for the "condition failed" case
>>
>> In particular in this bit:
>> if (dc->condjmp || !dc->is_jmp) {
>> gen_set_pc_im(dc, dc->pc);
>> dc->condjmp = 0;
>
> Hi Peter,
>
> Thank you a lot for your explanation! It was really helpful for
> understanding the code :) One thing I wasn't sure of was whether this
> "dc->condjmp = 0" means that "condition failed" codepath below will also
> generate an exception whereas it shouldn't?
You want a singlestep exception for both conditional-insn
failed and conditional-insn passed (either way we've executed
the instruction and should return control to the debugger).
(The code is I think more confusing than it needs to be
and also somewhat repetitive in the way we have the same code for
"handle a trap insn if condjmp" and "handle a trap insn if
not condjmp".)
> Getting into the way the condexec bits handled I see that
> gen_set_condexec() should be called before
> gen_helper_check_breakpoints(), and probably also before
> gen_helper_access_check_cp_reg() because these helpers can raise an
> exception. I'm going to prepare patches for that soon.
Yes, I think this is right in both cases. (I'm kind of
surprised that we haven't noticed the invalid condexec
for conditional Thumb mode system register accesses, but
I guess mostly guests don't try to access registers that
are going to trap.)
thanks
-- PMM
