On 9/27/19 3:30 AM, David Hildenbrand wrote:
>> + /* Update ILEN, except for breakpoint, where we didn't load an insn. */
>> + if (ilen) {
>> + env->int_pgm_ilen = ilen;
>> + }
>
> I am not completely sure about breakpoint handling and which
> implications we'll have when not setting int_pgm_ilen ...
Yeah. Possibly to make it simple I should simply assign 2 as the length of a
breakpoint, bearing in mind that there is no a s390 exception to be delivered
-- this is purely a qemu-internal thing, raising EXCP_DEBUG to get back to the
gdbstub interface.
> I wonder if that change can help to better handle exceptions during
> EXECUTE, whereby we have to indicate the EXECUTE instruction and the
> ilen of the EXECUTE instruction (so the pc and ilen of the original
> EXECUTE function, not of the EXECUTE target).
Yes, that's already there. The ilen of the execute insn is placed in the low 4
bits of env->ex_value, and that's what we record as ilen within extract_insn().
> I don't completely like the current interrupt handling when we have
> "env->ex_value" in "s390_cpu_exec_interrupt()". I'd love to see that
> check go, then we can reuse that function easily e.g., in MVC to test
> and inject exceptions while processing such an interruptible instruction
> - like MVC.
I don't think that reusing s390_cpu_exec_interrupt directly is a good idea.
There's other cleanup that needs to happen when exiting a TB.
What I think you should do instead is check env_neg(env)->icount_decr, exactly
like we do at the start of every basic block, and use that as an indication
that you should exit back to the main loop.
r~
