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