Re: [PATCH v3 02/18] target/s390x: Add ilen to unwind data

[David Hildenbrand <david@redhat.com>]

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