On 27.09.19 18:02, Richard Henderson wrote:
> 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().
Ah, good to know. I'll have to review PGM injection, which ILEN we
currently indicate and which one we are supposed to indicate.
>
>> 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.
The issue is that when we return to the main loop we really have to
inject an interrupt - otherwise we might simply skip parts of the
(interruptible) instruction and continue with the next one.
However, with I/O interrupts, we can actually race against other VCPUs.
So the I/O interrupt might be gone by the time we arrive in the main loop.
--
Thanks,
David / dhildenb
