Message ID | 20200310090950.61172-1-frankja@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v8] s390x: ipl: Consolidate iplb validity check into one function | expand |
On 10.03.20 10:09, 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> > Reviewed-by: David Hildenbrand <david@redhat.com> David, still ok? If yes I will pick this one. Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/s390x/ipl.h | 18 +++++++++--------- > target/s390x/diag.c | 2 +- > 2 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h > index d4813105db..3e44abe1c6 100644 > --- a/hw/s390x/ipl.h > +++ b/hw/s390x/ipl.h > @@ -173,16 +173,16 @@ 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; > + case S390_IPL_TYPE_CCW: > + return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN; > + 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; > } >
On 10.03.20 10:11, Christian Borntraeger wrote: > > > On 10.03.20 10:09, 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> >> Reviewed-by: David Hildenbrand <david@redhat.com> > > David, still ok? If yes I will pick this one. Sure, only missed the duplicate check. thanks > > > Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
On 10.03.20 10:09, 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> > Reviewed-by: David Hildenbrand <david@redhat.com> thanks applied.
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index d4813105db..3e44abe1c6 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -173,16 +173,16 @@ 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; + case S390_IPL_TYPE_CCW: + return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN; + 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; }