MVCL is interruptible and we should check for interrupts and process
them after writing back the variables to the registers. Let's check
for any exit requests and exit to the main loop.

When booting Fedora 30, I can see a handful of these exits and it seems
to work reliable. (it never get's triggered via EXECUTE, though)

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---

v1 -> v2:
- Check only if icount_decr.u32 < 0
- Drop should_interrupt_instruction() and perform the check inline
- Rephrase comment, subject, and description

---
 target/s390x/mem_helper.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4254548935..87e4ebd169 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1015,6 +1015,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
     uint64_t src = get_address(env, r2);
     uint8_t pad = env->regs[r2 + 1] >> 24;
+    CPUState *cs = env_cpu(env);
     S390Access srca, desta;
     uint32_t cc, cur_len;
 
@@ -1065,7 +1066,14 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
         env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
         set_address_zero(env, r1, dest);
 
-        /* TODO: Deliver interrupts. */
+        /*
+         * MVCL is interruptible. Check if somebody (e.g., cpu_interrupt() or
+         * cpu_exit()) asked us to return to the main loop. In case there is
+         * no deliverable interrupt, we'll end up back in this handler.
+         */
+        if (unlikely((int32_t)atomic_read(&cpu_neg(cs)->icount_decr.u32) < 0)) {
+            cpu_loop_exit_restore(cs, ra);
+        }
     }
     return cc;
 }
-- 
2.21.0

David Hildenbrand <david@redhat.com> writes:

> MVCL is interruptible and we should check for interrupts and process
> them after writing back the variables to the registers. Let's check
> for any exit requests and exit to the main loop.
>
> When booting Fedora 30, I can see a handful of these exits and it seems
> to work reliable. (it never get's triggered via EXECUTE, though)
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>
> v1 -> v2:
> - Check only if icount_decr.u32 < 0
> - Drop should_interrupt_instruction() and perform the check inline
> - Rephrase comment, subject, and description
>
> ---
>  target/s390x/mem_helper.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 4254548935..87e4ebd169 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -1015,6 +1015,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
>      uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
>      uint64_t src = get_address(env, r2);
>      uint8_t pad = env->regs[r2 + 1] >> 24;
> +    CPUState *cs = env_cpu(env);
>      S390Access srca, desta;
>      uint32_t cc, cur_len;
>
> @@ -1065,7 +1066,14 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
>          env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
>          set_address_zero(env, r1, dest);
>
> -        /* TODO: Deliver interrupts. */
> +        /*
> +         * MVCL is interruptible. Check if somebody (e.g., cpu_interrupt() or
> +         * cpu_exit()) asked us to return to the main loop. In case there is
> +         * no deliverable interrupt, we'll end up back in this handler.
> +         */
> +        if
> (unlikely((int32_t)atomic_read(&cpu_neg(cs)->icount_decr.u32) < 0)) {

I'm not sure about directly checking the icount_decr here. It really is
an internal implementation detail for the generated code. Having said
that is seems cpu_interrupt() is messing with this directly rather than
calling cpu_exit() which sets the more easily checked &cpu->exit_request.

This is potentially problematic as in other points in the cpu loop code
you see checks like this:

    /* Finally, check if we need to exit to the main loop.  */
    if (unlikely(atomic_read(&cpu->exit_request))
        || (use_icount
            && cpu_neg(cpu)->icount_decr.u16.low + cpu->icount_extra == 0)) {
        atomic_set(&cpu->exit_request, 0);
        if (cpu->exception_index == -1) {
            cpu->exception_index = EXCP_INTERRUPT;
        }
        return true;
    }

although I guess this is because interrupts and "exits" take subtly
different paths through the outer loop. Given that exits and interrupts
are slightly different is what you want to check
atomic_read(&cpu->interrupt_request))?

> +            cpu_loop_exit_restore(cs, ra);
> +        }
>      }
>      return cc;
>  }


--
Alex Bennée

On 10/2/19 2:58 AM, Alex Bennée wrote:
> 
> David Hildenbrand <david@redhat.com> writes:
> 
>> MVCL is interruptible and we should check for interrupts and process
>> them after writing back the variables to the registers. Let's check
>> for any exit requests and exit to the main loop.
>>
>> When booting Fedora 30, I can see a handful of these exits and it seems
>> to work reliable. (it never get's triggered via EXECUTE, though)
>>
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>
>> v1 -> v2:
>> - Check only if icount_decr.u32 < 0
>> - Drop should_interrupt_instruction() and perform the check inline
>> - Rephrase comment, subject, and description
>>
>> ---
>>  target/s390x/mem_helper.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
>> index 4254548935..87e4ebd169 100644
>> --- a/target/s390x/mem_helper.c
>> +++ b/target/s390x/mem_helper.c
>> @@ -1015,6 +1015,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
>>      uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
>>      uint64_t src = get_address(env, r2);
>>      uint8_t pad = env->regs[r2 + 1] >> 24;
>> +    CPUState *cs = env_cpu(env);
>>      S390Access srca, desta;
>>      uint32_t cc, cur_len;
>>
>> @@ -1065,7 +1066,14 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
>>          env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
>>          set_address_zero(env, r1, dest);
>>
>> -        /* TODO: Deliver interrupts. */
>> +        /*
>> +         * MVCL is interruptible. Check if somebody (e.g., cpu_interrupt() or
>> +         * cpu_exit()) asked us to return to the main loop. In case there is
>> +         * no deliverable interrupt, we'll end up back in this handler.
>> +         */
>> +        if
>> (unlikely((int32_t)atomic_read(&cpu_neg(cs)->icount_decr.u32) < 0)) {
> 
> I'm not sure about directly checking the icount_decr here. It really is
> an internal implementation detail for the generated code.

But it's also the exact right thing to test.


> Having said
> that is seems cpu_interrupt() is messing with this directly rather than
> calling cpu_exit() which sets the more easily checked &cpu->exit_request.
> 
> This is potentially problematic as in other points in the cpu loop code
> you see checks like this:
> 
>     /* Finally, check if we need to exit to the main loop.  */
>     if (unlikely(atomic_read(&cpu->exit_request))
>         || (use_icount
>             && cpu_neg(cpu)->icount_decr.u16.low + cpu->icount_extra == 0)) {
>         atomic_set(&cpu->exit_request, 0);
>         if (cpu->exception_index == -1) {
>             cpu->exception_index = EXCP_INTERRUPT;
>         }
>         return true;
>     }
> 
> although I guess this is because interrupts and "exits" take subtly
> different paths through the outer loop. Given that exits and interrupts
> are slightly different is what you want to check
> atomic_read(&cpu->interrupt_request))?

No, this is not about interrupts per se.

The thing we're trying to solve here is MVCL running for a long time.  The
length operand is 24 bits, so max 16MB can be copied with one instruction.  We
want to exit back to the main loop early when told to do so, as the insn is
officially restartable.

Ordinarily, I would say move the loop out to the tcg level, but that creates
further complications and I'd rather not open up that can of worms.

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.


r~

On 02.10.19 18:47, Richard Henderson wrote:
> On 10/2/19 2:58 AM, Alex Bennée wrote:
>>
>> David Hildenbrand <david@redhat.com> writes:
>>
>>> MVCL is interruptible and we should check for interrupts and process
>>> them after writing back the variables to the registers. Let's check
>>> for any exit requests and exit to the main loop.
>>>
>>> When booting Fedora 30, I can see a handful of these exits and it seems
>>> to work reliable. (it never get's triggered via EXECUTE, though)
>>>
>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>>
>>> v1 -> v2:
>>> - Check only if icount_decr.u32 < 0
>>> - Drop should_interrupt_instruction() and perform the check inline
>>> - Rephrase comment, subject, and description
>>>
>>> ---
>>>  target/s390x/mem_helper.c | 10 +++++++++-
>>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
>>> index 4254548935..87e4ebd169 100644
>>> --- a/target/s390x/mem_helper.c
>>> +++ b/target/s390x/mem_helper.c
>>> @@ -1015,6 +1015,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
>>>      uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
>>>      uint64_t src = get_address(env, r2);
>>>      uint8_t pad = env->regs[r2 + 1] >> 24;
>>> +    CPUState *cs = env_cpu(env);
>>>      S390Access srca, desta;
>>>      uint32_t cc, cur_len;
>>>
>>> @@ -1065,7 +1066,14 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
>>>          env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
>>>          set_address_zero(env, r1, dest);
>>>
>>> -        /* TODO: Deliver interrupts. */
>>> +        /*
>>> +         * MVCL is interruptible. Check if somebody (e.g., cpu_interrupt() or
>>> +         * cpu_exit()) asked us to return to the main loop. In case there is
>>> +         * no deliverable interrupt, we'll end up back in this handler.
>>> +         */
>>> +        if
>>> (unlikely((int32_t)atomic_read(&cpu_neg(cs)->icount_decr.u32) < 0)) {
>>
>> I'm not sure about directly checking the icount_decr here. It really is
>> an internal implementation detail for the generated code.
> 
> But it's also the exact right thing to test.
> 
> 
>> Having said
>> that is seems cpu_interrupt() is messing with this directly rather than
>> calling cpu_exit() which sets the more easily checked &cpu->exit_request.
>>
>> This is potentially problematic as in other points in the cpu loop code
>> you see checks like this:
>>
>>     /* Finally, check if we need to exit to the main loop.  */
>>     if (unlikely(atomic_read(&cpu->exit_request))
>>         || (use_icount
>>             && cpu_neg(cpu)->icount_decr.u16.low + cpu->icount_extra == 0)) {
>>         atomic_set(&cpu->exit_request, 0);
>>         if (cpu->exception_index == -1) {
>>             cpu->exception_index = EXCP_INTERRUPT;
>>         }
>>         return true;
>>     }
>>
>> although I guess this is because interrupts and "exits" take subtly
>> different paths through the outer loop. Given that exits and interrupts
>> are slightly different is what you want to check
>> atomic_read(&cpu->interrupt_request))?
> 
> No, this is not about interrupts per se.
> 
> The thing we're trying to solve here is MVCL running for a long time.  The
> length operand is 24 bits, so max 16MB can be copied with one instruction.  We
> want to exit back to the main loop early when told to do so, as the insn is
> officially restartable.
> 
> Ordinarily, I would say move the loop out to the tcg level, but that creates
> further complications and I'd rather not open up that can of worms.

While that is feasible, I agree that it's not the simplest approach.

> 
> 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.

We could rap that in something like

cpu_cond_loop_exit_restore()

inspired by cond_resched() in the kernel. Then, at least the
implementation specifics are kept where they actually belong.

-- 

Thanks,

David / dhildenb

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.

(3b) helper_mvcl() exits to main loop,

     - cpu_loop_exit_restore() unwinds psw.addr = Ae.
     - Next instruction is the EXECUTE itself...
     - goto 1.

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.


r~

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

David Hildenbrand <david@redhat.com> writes:

> 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?

It would be nice to avoid this internal detail in the guest specific
code but I can live with it for now.

Acked-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée

On Fri, 4 Oct 2019 at 09:02, David Hildenbrand <david@redhat.com> wrote:
> 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?

Nothing else in target code touches the icount data structures,
so if this s390 insn needs to make this check I think it ought
to do it by calling a function implemented by the tcg code;
that can then have a good name that describes what it's doing
and a doc comment explaining the reason we need to have it.

thanks
-- PMM

On 04.10.19 14:11, Peter Maydell wrote:
> On Fri, 4 Oct 2019 at 09:02, David Hildenbrand <david@redhat.com> wrote:
>> 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?
> 
> Nothing else in target code touches the icount data structures,
> so if this s390 insn needs to make this check I think it ought
> to do it by calling a function implemented by the tcg code;
> that can then have a good name that describes what it's doing
> and a doc comment explaining the reason we need to have it.
> 
> thanks
> -- PMM
> 

I can offer something like this:

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 49db07ba0b..d370ac0134 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -72,6 +72,26 @@ void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
 void QEMU_NORETURN cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc);
 
+/**
+ * cpu_cond_loop_exit_restore:
+ * @cpu: the vCPU state to be restored
+ * @pc: the host PC
+ *
+ * Trigger a cpu_loop_exit_restore() in case somebody asked for a return
+ * to the main loop (e.g. cpu_exit() or cpu_interrupt()).
+ *
+ * This is helpful for architectures that support interruptible
+ * instructions. After writing back all state to registers/memory, this
+ * call can be used to conditionally return back to the main loop or to
+ * continue executing the interruptible instruction.
+ */
+static inline void cpu_cond_loop_exit_restore(CPUState *cpu, uintptr_t pc)
+{
+    if (unlikely((int32_t)atomic_read(&cpu_neg(cs)->icount_decr.u32) < 0)) {
+        cpu_loop_exit_restore(cs, ra);
+    }
+}
+
 #if !defined(CONFIG_USER_ONLY)
 void cpu_reloading_memory_map(void);
 /**


Or, as alternative, something like "cpu_shall_exit()" which only
wraps the single check.

-- 

Thanks,

David / dhildenb

On 10/4/19 5:34 AM, David Hildenbrand wrote:
> Or, as alternative, something like "cpu_shall_exit()" which only
> wraps the single check.

I would prefer this to the full cpu_loop_exit_restore.
It's harder to imagine what else might need doing for
some other user of the interface.


r~

David Hildenbrand <david@redhat.com> writes:

> On 04.10.19 14:11, Peter Maydell wrote:
>> On Fri, 4 Oct 2019 at 09:02, David Hildenbrand <david@redhat.com> wrote:
>>> 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?
>>
>> Nothing else in target code touches the icount data structures,
>> so if this s390 insn needs to make this check I think it ought
>> to do it by calling a function implemented by the tcg code;
>> that can then have a good name that describes what it's doing
>> and a doc comment explaining the reason we need to have it.
>>
>> thanks
>> -- PMM
>>
>
> I can offer something like this:
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 49db07ba0b..d370ac0134 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -72,6 +72,26 @@ void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
>  void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
>  void QEMU_NORETURN cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc);
>
> +/**
> + * cpu_cond_loop_exit_restore:
> + * @cpu: the vCPU state to be restored
> + * @pc: the host PC
> + *
> + * Trigger a cpu_loop_exit_restore() in case somebody asked for a return
> + * to the main loop (e.g. cpu_exit() or cpu_interrupt()).
> + *
> + * This is helpful for architectures that support interruptible
> + * instructions. After writing back all state to registers/memory, this
> + * call can be used to conditionally return back to the main loop or to
> + * continue executing the interruptible instruction.
> + */
> +static inline void cpu_cond_loop_exit_restore(CPUState *cpu, uintptr_t pc)
> +{
> +    if (unlikely((int32_t)atomic_read(&cpu_neg(cs)->icount_decr.u32) < 0)) {
> +        cpu_loop_exit_restore(cs, ra);
> +    }
> +}
> +

cpu_loop_exit_restore_cond_irq
cpu_loop_exit_restore_ifirq
cpu_loop_exit_restore_conditional
cpu_loop_exit_restore_maybe

meh, naming stuff is hard:

Acked-by: Alex Bennée <alex.bennee@linaro.org>

>  #if !defined(CONFIG_USER_ONLY)
>  void cpu_reloading_memory_map(void);
>  /**
>
>
> Or, as alternative, something like "cpu_shall_exit()" which only
> wraps the single check.


--
Alex Bennée