| Message ID | 20210813073615.32837-5-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Cleanup and maintenance | expand |
On Fri, 13 Aug 2021 07:36:11 +0000 Janosch Frank <frankja@linux.ibm.com> wrote: > TEID data is specified in the Principles of Operation as bits so it > makes more sens to test the bits instead of anding the mask. > > We need to set -Wno-address-of-packed-member since for test bit we > take an address of a struct lowcore member. and s390x has no alignment requirements > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > lib/s390x/interrupt.c | 5 +++-- > s390x/Makefile | 1 + > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c > index 1248bceb..e05c212e 100644 > --- a/lib/s390x/interrupt.c > +++ b/lib/s390x/interrupt.c > @@ -8,6 +8,7 @@ > * David Hildenbrand <david@redhat.com> > */ > #include <libcflat.h> > +#include <bitops.h> > #include <asm/barrier.h> > #include <sclp.h> > #include <interrupt.h> > @@ -77,8 +78,8 @@ static void fixup_pgm_int(struct stack_frame_int > *stack) break; > case PGM_INT_CODE_PROTECTION: > /* Handling for iep.c test case. */ > - if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & > 0x04UL && > - !(lc->trans_exc_id & 0x08UL)) > + if (test_bit_inv(56, &lc->trans_exc_id) && > test_bit_inv(61, &lc->trans_exc_id) && > + !test_bit_inv(60, &lc->trans_exc_id)) > /* > * We branched to the instruction that caused > * the exception so we can use the return > diff --git a/s390x/Makefile b/s390x/Makefile > index ef8041a6..d260b336 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -45,6 +45,7 @@ CFLAGS += -O2 > CFLAGS += -march=zEC12 > CFLAGS += -mbackchain > CFLAGS += -fno-delete-null-pointer-checks > +CFLAGS += -Wno-address-of-packed-member > LDFLAGS += -nostdlib -Wl,--build-id=none > > # We want to keep intermediate files
On 13/08/2021 09.36, Janosch Frank wrote: > TEID data is specified in the Principles of Operation as bits so it > makes more sens to test the bits instead of anding the mask. > > We need to set -Wno-address-of-packed-member since for test bit we > take an address of a struct lowcore member. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/interrupt.c | 5 +++-- > s390x/Makefile | 1 + > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c > index 1248bceb..e05c212e 100644 > --- a/lib/s390x/interrupt.c > +++ b/lib/s390x/interrupt.c > @@ -8,6 +8,7 @@ > * David Hildenbrand <david@redhat.com> > */ > #include <libcflat.h> > +#include <bitops.h> > #include <asm/barrier.h> > #include <sclp.h> > #include <interrupt.h> > @@ -77,8 +78,8 @@ static void fixup_pgm_int(struct stack_frame_int *stack) > break; > case PGM_INT_CODE_PROTECTION: > /* Handling for iep.c test case. */ > - if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL && > - !(lc->trans_exc_id & 0x08UL)) > + if (test_bit_inv(56, &lc->trans_exc_id) && test_bit_inv(61, &lc->trans_exc_id) && > + !test_bit_inv(60, &lc->trans_exc_id)) I'd rather prefer: if ((lc->trans_exc_id & 0x8c) == 0x84) ... or maybe you could add a helper function for these checks a la: bool check_teid_prot_cause(uint64_t teid, bool bit56, bool bit60, bool bit61) then you could replace the if statement with: if (check_teid_prot_cause(lc->trans_exc_id, 1, 0, 1)) which would be way more readable, IMHO. > /* > * We branched to the instruction that caused > * the exception so we can use the return > diff --git a/s390x/Makefile b/s390x/Makefile > index ef8041a6..d260b336 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -45,6 +45,7 @@ CFLAGS += -O2 > CFLAGS += -march=zEC12 > CFLAGS += -mbackchain > CFLAGS += -fno-delete-null-pointer-checks > +CFLAGS += -Wno-address-of-packed-member I think we should avoid this since this also affects the common code, doesn't it? And in common code, we might need to deal with this. Thomas
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c index 1248bceb..e05c212e 100644 --- a/lib/s390x/interrupt.c +++ b/lib/s390x/interrupt.c @@ -8,6 +8,7 @@ * David Hildenbrand <david@redhat.com> */ #include <libcflat.h> +#include <bitops.h> #include <asm/barrier.h> #include <sclp.h> #include <interrupt.h> @@ -77,8 +78,8 @@ static void fixup_pgm_int(struct stack_frame_int *stack) break; case PGM_INT_CODE_PROTECTION: /* Handling for iep.c test case. */ - if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL && - !(lc->trans_exc_id & 0x08UL)) + if (test_bit_inv(56, &lc->trans_exc_id) && test_bit_inv(61, &lc->trans_exc_id) && + !test_bit_inv(60, &lc->trans_exc_id)) /* * We branched to the instruction that caused * the exception so we can use the return diff --git a/s390x/Makefile b/s390x/Makefile index ef8041a6..d260b336 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -45,6 +45,7 @@ CFLAGS += -O2 CFLAGS += -march=zEC12 CFLAGS += -mbackchain CFLAGS += -fno-delete-null-pointer-checks +CFLAGS += -Wno-address-of-packed-member LDFLAGS += -nostdlib -Wl,--build-id=none # We want to keep intermediate files
TEID data is specified in the Principles of Operation as bits so it makes more sens to test the bits instead of anding the mask. We need to set -Wno-address-of-packed-member since for test bit we take an address of a struct lowcore member. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- lib/s390x/interrupt.c | 5 +++-- s390x/Makefile | 1 + 2 files changed, 4 insertions(+), 2 deletions(-)