Re: [PATCH v4 6/6] s390x: kvm: Make kvm_sclp_service_call void

[David Hildenbrand]

On 27.11.19 18:50, Janosch Frank wrote:
> It defaults to returning 0 anyway and that return value is not
> necessary, as 0 is also the default rc that the caller would return.
> 
> While doing that we can simplify the logic a bit and return early if
> we inject a PGM exception. Also we always set a 0 cc, so let's not
> base it on the rc of the sclp emulation functions.
> 
> Signed-off-by: Janosch Frank <address@hidden>
> ---
>  target/s390x/kvm.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 0c9d14b4b1..08bb1edca0 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1159,13 +1159,13 @@ void kvm_s390_access_exception(S390CPU *cpu, uint16_t 
> code, uint64_t te_code)
>      kvm_s390_vcpu_interrupt(cpu, &irq);
>  }
>  
> -static int kvm_sclp_service_call(S390CPU *cpu, struct kvm_run *run,
> +static void kvm_sclp_service_call(S390CPU *cpu, struct kvm_run *run,
>                                   uint16_t ipbh0)
>  {
>      CPUS390XState *env = &cpu->env;
>      uint64_t sccb;
>      uint32_t code;
> -    int r = 0;
> +    int r;
>  
>      sccb = env->regs[ipbh0 & 0xf];
>      code = env->regs[(ipbh0 & 0xf0) >> 4];
> @@ -1173,11 +1173,9 @@ static int kvm_sclp_service_call(S390CPU *cpu, struct 
> kvm_run *run,
>      r = sclp_service_call(env, sccb, code);
>      if (r < 0) {
>          kvm_s390_program_interrupt(cpu, -r);
> -    } else {
> -        setcc(cpu, r);
> +        return;
>      }
> -
> -    return 0;
> +    setcc(cpu, 0);

For now, sclp_service_call will return <= 0 ... but don't we actually
have the option to return a cc? What does the spec say? Always set to 0?

At least also the TCG implementation sets the CC to whatever is returned
here .... and Claudio's unit tests have code to handle cc != 0 ... and
the kernel as well (drivers/s390/char/sclp.h:sclp_service_call())



-- 
Thanks,

David / dhildenb