Re: [PATCH v2] s390x/tcg: MVCL: Exit to main loop if requested

[David Hildenbrand <david@redhat.com>]

On 02.10.19 21:34, Richard Henderson wrote:
> On 10/2/19 9:47 AM, Richard Henderson wrote:
>> There is still the special case of EXECUTE of MVCL, which I suspect must have
>> some failure mode that we're not considering -- the setting and clearing of
>> ex_value can't help.  I have a suspicion that we need to special case that
>> within helper_ex, just so that ex_value doesn't enter into it.
> 
> I had a walk and a think.  I now believe that we're ok:
> 
> (1) TB with EXECUTE runs, at address Ae
> 
>     - env->psw_addr stored with Ae.
>     - helper_ex() runs, memory address Am computed
>       from D2a(X2a,B2a) or from psw.addr+RI2.
>     - env->ex_value stored with memory value modified by R1a
> 
> (2) TB of executee runs,
> 
>     - env->ex_value stored with 0.
>     - helper_mvcl() runs, using and updating R1b, R1b+1, R2b, R2b+1.
> 
> (3a) helper_mvcl() completes,
> 
>      - TB of executee continues, psw.addr += ilen.
>      - Next instruction is the one following EXECUTE.

Right, and whenever we inject an interrupt (e.g., access exception or an
asynchronous one), we have to use addr=ilen of EXECUTE, not of the
execute target. And that is guaranteed to reside in env->psw_addr/rewind
info

> 
> (3b) helper_mvcl() exits to main loop,
> 
>      - cpu_loop_exit_restore() unwinds psw.addr = Ae.
>      - Next instruction is the EXECUTE itself...
>      - goto 1.

Sounds about right to me. Thanks for verifying.

> 
> If we can agree that the result is undefined if registers R1a, X2a, B2a overlap
> R1b, R1b+1, R2b, R2b+1, or if the memory address Am is modified by the
> interrupted MVCL, then we're ok.

So the Programming Note 5. for EXECUTE says:

When an interruptible instruction is made the tar-
get of an execute-type instruction, the program
normally should not designate any register
updated by the interruptible instruction as the R1,
X2, or B2 register for EXECUTE or R1 register for
EXECUTE RELATIVE LONG. Otherwise, on
resumption of execution after an interruption, or if
the instruction is refetched without an interrup-
tion, the updated values of these registers will be
used in the execution of the execute-type instruc-
tion. Similarly, the program should normally not
let the destination field in storage of an interrupt-
ible instruction include the location of an execute-
type instruction, since the new contents of the
location may be interpreted when resuming exe-
cution.

So, if I read correctly
- The updated register content *will* be used
- The updated memory content *may* be used

However, also "the program normally should not"/"the program should
normally not" which to me sounds like "just don't do it", in which case
we are fine.

So shall we leave this patch as-is (adding a summary of what you
explained to the description) or shall we somehow factor out the
TCG-internal-thingy check?

-- 

Thanks,

David / dhildenb