Re: [RFC PATCH v5 1/1] s390x: sigp: Reorder the SIGP STOP code

[David Hildenbrand]

On 13.12.21 22:09, Eric Farman wrote:
> Let's wait to mark the VCPU STOPPED until the possible
> STORE STATUS operation is completed, so that we know the
> CPU is fully stopped and done doing anything. (When we
> also clear the possible sigp_order field for STOP orders.)
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  target/s390x/sigp.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
> index 51c727834c..9dd977349a 100644
> --- a/target/s390x/sigp.c
> +++ b/target/s390x/sigp.c
> @@ -139,7 +139,7 @@ static void sigp_stop_and_store_status(CPUState *cs, 
> run_on_cpu_data arg)
>      case S390_CPU_STATE_OPERATING:
>          cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
>          cpu_inject_stop(cpu);
> -        /* store will be performed in do_stop_interrup() */
> +        /* store will be performed in do_stop_interrupt() */
>          break;
>      case S390_CPU_STATE_STOPPED:
>          /* already stopped, just store the status */
> @@ -479,13 +479,17 @@ void do_stop_interrupt(CPUS390XState *env)
>  {
>      S390CPU *cpu = env_archcpu(env);
>  
> -    if (s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu) == 0) {
> -        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> -    }
> +    /*
> +     * Complete the STOP operation before exposing the CPU as
> +     * STOPPED to the system.
> +     */
>      if (cpu->env.sigp_order == SIGP_STOP_STORE_STATUS) {
>          s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
>      }
>      env->sigp_order = 0;
> +    if (s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu) == 0) {
> +        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +    }
>      env->pending_int &= ~INTERRUPT_STOP;
>  }
>  

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb