On 14.11.2015 22:45, Peter Maydell wrote:
> 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).
To be clear, I mean SWI/HVC/SMC exceptions could be generated when
singlestepping through a conditional-insn failed codepath.
>
> (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".)
I think I could make this code more clear and concise as well as put
some comments to describe it :)
Best regards,
Sergey
>
>> 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.)
