Re: [RFC PATCH v1 1/2] s390x: sigp: Force Set Architecture to return Invalid Parameter

[Thomas Huth <thuth@redhat.com>]

On 08/10/2021 22.38, Eric Farman wrote:
> According to the Principles of Operation, the SIGP Set Architecture
> order will return Incorrect State if some CPUs are not stopped, but
> only if the CZAM facility is not present. If it is, the order will
> return Invalid Parameter because the architecture mode cannot be
> changed.
> 
> Since CZAM always exists when S390_FEAT_ZARCH exists, which in turn
> exists for every defined CPU model, we can simplify this code.
> 
> Fixes: 075e52b81664 ("s390x/cpumodel: we are always in zarchitecture mode")
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   target/s390x/sigp.c | 18 +-----------------
>   1 file changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
> index d57427ced8..51c727834c 100644
> --- a/target/s390x/sigp.c
> +++ b/target/s390x/sigp.c
> @@ -428,26 +428,10 @@ static int handle_sigp_single_dst(S390CPU *cpu, S390CPU *dst_cpu, uint8_t order,
>   static int sigp_set_architecture(S390CPU *cpu, uint32_t param,
>                                    uint64_t *status_reg)
>   {
> -    CPUState *cur_cs;
> -    S390CPU *cur_cpu;
> -    bool all_stopped = true;
> -
> -    CPU_FOREACH(cur_cs) {
> -        cur_cpu = S390_CPU(cur_cs);
> -
> -        if (cur_cpu == cpu) {
> -            continue;
> -        }
> -        if (s390_cpu_get_state(cur_cpu) != S390_CPU_STATE_STOPPED) {
> -            all_stopped = false;
> -        }
> -    }
> -
>       *status_reg &= 0xffffffff00000000ULL;
>   
>       /* Reject set arch order, with czam we're always in z/Arch mode. */
> -    *status_reg |= (all_stopped ? SIGP_STAT_INVALID_PARAMETER :
> -                    SIGP_STAT_INCORRECT_STATE);
> +    *status_reg |= SIGP_STAT_INVALID_PARAMETER;
>       return SIGP_CC_STATUS_STORED;
>   }

Reviewed-by: Thomas Huth <thuth@redhat.com>

By the way, I think we could now also get rid of SIGP_MODE_ESA_S390, 
SIGP_MODE_Z_ARCH_TRANS_ALL_PSW and SIGP_MODE_Z_ARCH_TRANS_CUR_PSW now (in a 
separate patch)...