Message ID | 20200310083232.29805-1-frankja@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [v8,1/2] s390x: ipl: Consolidate iplb validity check into one function | expand |
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 <frankja@linux.ibm.com> > --- > 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 <david@redhat.com>
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 <frankja@linux.ibm.com> > --- > 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); Isnt the iplb->pbt check redundant due to the switch statement? > + case S390_IPL_TYPE_CCW: > + return (be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN && > + iplb->pbt == S390_IPL_TYPE_CCW); > + default: same here.
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; }
It's nicer to just call one function than calling a function for each possible iplb type. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- hw/s390x/ipl.h | 20 +++++++++++--------- target/s390x/diag.c | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-)