Re: [PATCH v8 1/2] s390x: ipl: Consolidate iplb validity check into one function

[David Hildenbrand]

On 10.03.20 09:32, Janosch Frank wrote:
> It's nicer to just call one function than calling a function for each
> possible iplb type.
> 
> Signed-off-by: Janosch Frank <address@hidden>
> ---
>  hw/s390x/ipl.h      | 20 +++++++++++---------
>  target/s390x/diag.c |  2 +-
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index d4813105db..211ce2dbeb 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -173,16 +173,18 @@ static inline bool iplb_valid_len(IplParameterBlock 
> *iplb)
>      return be32_to_cpu(iplb->len) <= sizeof(IplParameterBlock);
>  }
>  
> -static inline bool iplb_valid_ccw(IplParameterBlock *iplb)
> +static inline bool iplb_valid(IplParameterBlock *iplb)
>  {
> -    return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN &&
> -           iplb->pbt == S390_IPL_TYPE_CCW;
> -}
> -
> -static inline bool iplb_valid_fcp(IplParameterBlock *iplb)
> -{
> -    return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_FCP_LEN &&
> -           iplb->pbt == S390_IPL_TYPE_FCP;
> +    switch (iplb->pbt) {
> +    case S390_IPL_TYPE_FCP:
> +        return (be32_to_cpu(iplb->len) >= S390_IPLB_MIN_FCP_LEN &&
> +                iplb->pbt == S390_IPL_TYPE_FCP);
> +    case S390_IPL_TYPE_CCW:
> +        return (be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN &&
> +                iplb->pbt == S390_IPL_TYPE_CCW);
> +    default:
> +        return false;
> +    }
>  }
>  
>  #endif
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index b5aec06d6b..54e5670b3f 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -117,7 +117,7 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, 
> uint64_t r3, uintptr_t ra)
>  
>          cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len));
>  
> -        if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb)) {
> +        if (!iplb_valid(iplb)) {
>              env->regs[r1 + 1] = DIAG_308_RC_INVALID;
>              goto out;
>          }
> 

Reviewed-by: David Hildenbrand <address@hidden>

-- 
Thanks,

David / dhildenb