Message ID | 1591603981-16879-2-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | s390x: Testing the Channel Subsystem I/O | expand |
On 08/06/2020 10.12, Pierre Morel wrote: > This patch defines the PSW bits EA/BA used to initialize the PSW masks > for exceptions. > > Since some PSW mask definitions exist already in arch_def.h we add these > definitions there. > We move all PSW definitions together and protect assembler code against > C syntax. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/asm/arch_def.h | 15 +++++++++++---- > s390x/cstart64.S | 15 ++++++++------- > 2 files changed, 19 insertions(+), 11 deletions(-) > > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h > index 1b3bb0c..5388114 100644 > --- a/lib/s390x/asm/arch_def.h > +++ b/lib/s390x/asm/arch_def.h > @@ -10,15 +10,21 @@ > #ifndef _ASM_S390X_ARCH_DEF_H_ > #define _ASM_S390X_ARCH_DEF_H_ > > +#define PSW_MASK_EXT 0x0100000000000000UL > +#define PSW_MASK_DAT 0x0400000000000000UL > +#define PSW_MASK_SHORT_PSW 0x0008000000000000UL > +#define PSW_MASK_PSTATE 0x0001000000000000UL > +#define PSW_MASK_BA 0x0000000080000000UL > +#define PSW_MASK_EA 0x0000000100000000UL > + > +#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA) PSW_EXCEPTION_MASK sounds a little bit unfortunate - that term rather reminds me of something that disables some interrupts ... in case you respin, maybe rather use something like "PSW_EXC_ADDR_MODE" ? Well, since nobody else complained yet, and the rest of the patch looks fine: Acked-by: Thomas Huth <thuth@redhat.com>
On 2020-06-08 10:43, Thomas Huth wrote: > On 08/06/2020 10.12, Pierre Morel wrote: >> This patch defines the PSW bits EA/BA used to initialize the PSW masks >> for exceptions. >> >> Since some PSW mask definitions exist already in arch_def.h we add these >> definitions there. >> We move all PSW definitions together and protect assembler code against >> C syntax. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> lib/s390x/asm/arch_def.h | 15 +++++++++++---- >> s390x/cstart64.S | 15 ++++++++------- >> 2 files changed, 19 insertions(+), 11 deletions(-) >> >> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h >> index 1b3bb0c..5388114 100644 >> --- a/lib/s390x/asm/arch_def.h >> +++ b/lib/s390x/asm/arch_def.h >> @@ -10,15 +10,21 @@ >> #ifndef _ASM_S390X_ARCH_DEF_H_ >> #define _ASM_S390X_ARCH_DEF_H_ >> >> +#define PSW_MASK_EXT 0x0100000000000000UL >> +#define PSW_MASK_DAT 0x0400000000000000UL >> +#define PSW_MASK_SHORT_PSW 0x0008000000000000UL >> +#define PSW_MASK_PSTATE 0x0001000000000000UL >> +#define PSW_MASK_BA 0x0000000080000000UL >> +#define PSW_MASK_EA 0x0000000100000000UL >> + >> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA) > > PSW_EXCEPTION_MASK sounds a little bit unfortunate - that term rather > reminds me of something that disables some interrupts > ... in case you > respin, maybe rather use something like "PSW_EXC_ADDR_MODE" ? EXCEPTIONS_PSW_MASK ? a vote? > Well, since nobody else complained yet, and the rest of the patch looks > fine: > > Acked-by: Thomas Huth <thuth@redhat.com> > Thanks, Pierre
On 08/06/2020 16.33, Pierre Morel wrote: > > > On 2020-06-08 10:43, Thomas Huth wrote: >> On 08/06/2020 10.12, Pierre Morel wrote: >>> This patch defines the PSW bits EA/BA used to initialize the PSW masks >>> for exceptions. >>> >>> Since some PSW mask definitions exist already in arch_def.h we add these >>> definitions there. >>> We move all PSW definitions together and protect assembler code against >>> C syntax. >>> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> lib/s390x/asm/arch_def.h | 15 +++++++++++---- >>> s390x/cstart64.S | 15 ++++++++------- >>> 2 files changed, 19 insertions(+), 11 deletions(-) >>> >>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h >>> index 1b3bb0c..5388114 100644 >>> --- a/lib/s390x/asm/arch_def.h >>> +++ b/lib/s390x/asm/arch_def.h >>> @@ -10,15 +10,21 @@ >>> #ifndef _ASM_S390X_ARCH_DEF_H_ >>> #define _ASM_S390X_ARCH_DEF_H_ >>> +#define PSW_MASK_EXT 0x0100000000000000UL >>> +#define PSW_MASK_DAT 0x0400000000000000UL >>> +#define PSW_MASK_SHORT_PSW 0x0008000000000000UL >>> +#define PSW_MASK_PSTATE 0x0001000000000000UL >>> +#define PSW_MASK_BA 0x0000000080000000UL >>> +#define PSW_MASK_EA 0x0000000100000000UL >>> + >>> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA) >> >> PSW_EXCEPTION_MASK sounds a little bit unfortunate - that term rather >> reminds me of something that disables some interrupts >> ... in case you >> respin, maybe rather use something like "PSW_EXC_ADDR_MODE" ? > > EXCEPTIONS_PSW_MASK ? I think it is the _MASK suffix that mainly bugs me here, since this is not a define that you normally use for extracting the bits from a PSW... so EXCEPTIONS_PSW without _MASK would be fine for me... but as long as I'm the only one who has a strange feeling about this, it's also ok if you keep the current name. Thomas
On 2020-06-08 16:52, Thomas Huth wrote: > On 08/06/2020 16.33, Pierre Morel wrote: >> >> >> On 2020-06-08 10:43, Thomas Huth wrote: >>> On 08/06/2020 10.12, Pierre Morel wrote: >>>> This patch defines the PSW bits EA/BA used to initialize the PSW masks >>>> for exceptions. >>>> >>>> Since some PSW mask definitions exist already in arch_def.h we add these >>>> definitions there. >>>> We move all PSW definitions together and protect assembler code against >>>> C syntax. >>>> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> >>>> --- >>>> lib/s390x/asm/arch_def.h | 15 +++++++++++---- >>>> s390x/cstart64.S | 15 ++++++++------- >>>> 2 files changed, 19 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h >>>> index 1b3bb0c..5388114 100644 >>>> --- a/lib/s390x/asm/arch_def.h >>>> +++ b/lib/s390x/asm/arch_def.h >>>> @@ -10,15 +10,21 @@ >>>> #ifndef _ASM_S390X_ARCH_DEF_H_ >>>> #define _ASM_S390X_ARCH_DEF_H_ >>>> +#define PSW_MASK_EXT 0x0100000000000000UL >>>> +#define PSW_MASK_DAT 0x0400000000000000UL >>>> +#define PSW_MASK_SHORT_PSW 0x0008000000000000UL >>>> +#define PSW_MASK_PSTATE 0x0001000000000000UL >>>> +#define PSW_MASK_BA 0x0000000080000000UL >>>> +#define PSW_MASK_EA 0x0000000100000000UL >>>> + >>>> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA) >>> >>> PSW_EXCEPTION_MASK sounds a little bit unfortunate - that term rather >>> reminds me of something that disables some interrupts >>> ... in case you >>> respin, maybe rather use something like "PSW_EXC_ADDR_MODE" ? >> >> EXCEPTIONS_PSW_MASK ? > > I think it is the _MASK suffix that mainly bugs me here, since this is > not a define that you normally use for extracting the bits from a PSW... > so EXCEPTIONS_PSW without _MASK would be fine for me... but as long as > I'm the only one who has a strange feeling about this, it's also ok if > you keep the current name. > > Thomas > The _MASK is because it is applied to the psw.mask and not to the psw.addr part. But I agree that the name is not good, to keep the naming convention, may be it should be: PSW_MASK_ON_EXCEPTION beginning with PSW_MASK_ like all other psw.mask definitions and ON_EXCEPTION clearly define when it is used. Pierre
On 08/06/2020 17.28, Pierre Morel wrote: > > > On 2020-06-08 16:52, Thomas Huth wrote: >> On 08/06/2020 16.33, Pierre Morel wrote: >>> >>> >>> On 2020-06-08 10:43, Thomas Huth wrote: >>>> On 08/06/2020 10.12, Pierre Morel wrote: >>>>> This patch defines the PSW bits EA/BA used to initialize the PSW masks >>>>> for exceptions. >>>>> >>>>> Since some PSW mask definitions exist already in arch_def.h we add >>>>> these >>>>> definitions there. >>>>> We move all PSW definitions together and protect assembler code >>>>> against >>>>> C syntax. >>>>> >>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> >>>>> --- >>>>> lib/s390x/asm/arch_def.h | 15 +++++++++++---- >>>>> s390x/cstart64.S | 15 ++++++++------- >>>>> 2 files changed, 19 insertions(+), 11 deletions(-) >>>>> >>>>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h >>>>> index 1b3bb0c..5388114 100644 >>>>> --- a/lib/s390x/asm/arch_def.h >>>>> +++ b/lib/s390x/asm/arch_def.h >>>>> @@ -10,15 +10,21 @@ >>>>> #ifndef _ASM_S390X_ARCH_DEF_H_ >>>>> #define _ASM_S390X_ARCH_DEF_H_ >>>>> +#define PSW_MASK_EXT 0x0100000000000000UL >>>>> +#define PSW_MASK_DAT 0x0400000000000000UL >>>>> +#define PSW_MASK_SHORT_PSW 0x0008000000000000UL >>>>> +#define PSW_MASK_PSTATE 0x0001000000000000UL >>>>> +#define PSW_MASK_BA 0x0000000080000000UL >>>>> +#define PSW_MASK_EA 0x0000000100000000UL >>>>> + >>>>> +#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA) >>>> >>>> PSW_EXCEPTION_MASK sounds a little bit unfortunate - that term rather >>>> reminds me of something that disables some interrupts >>>> ... in case you >>>> respin, maybe rather use something like "PSW_EXC_ADDR_MODE" ? >>> >>> EXCEPTIONS_PSW_MASK ? >> >> I think it is the _MASK suffix that mainly bugs me here, since this is >> not a define that you normally use for extracting the bits from a PSW... >> so EXCEPTIONS_PSW without _MASK would be fine for me... but as long as >> I'm the only one who has a strange feeling about this, it's also ok if >> you keep the current name. >> >> Thomas >> > > The _MASK is because it is applied to the psw.mask and not to the > psw.addr part. > > But I agree that the name is not good, to keep the naming convention, > may be it should be: > > PSW_MASK_ON_EXCEPTION > > beginning with PSW_MASK_ like all other psw.mask definitions and > ON_EXCEPTION clearly define when it is used. Good idea, PSW_MASK_ON_EXCEPTION sounds better for me, too! Thomas
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h index 1b3bb0c..5388114 100644 --- a/lib/s390x/asm/arch_def.h +++ b/lib/s390x/asm/arch_def.h @@ -10,15 +10,21 @@ #ifndef _ASM_S390X_ARCH_DEF_H_ #define _ASM_S390X_ARCH_DEF_H_ +#define PSW_MASK_EXT 0x0100000000000000UL +#define PSW_MASK_DAT 0x0400000000000000UL +#define PSW_MASK_SHORT_PSW 0x0008000000000000UL +#define PSW_MASK_PSTATE 0x0001000000000000UL +#define PSW_MASK_BA 0x0000000080000000UL +#define PSW_MASK_EA 0x0000000100000000UL + +#define PSW_EXCEPTION_MASK (PSW_MASK_EA | PSW_MASK_BA) + +#ifndef __ASSEMBLER__ struct psw { uint64_t mask; uint64_t addr; }; -#define PSW_MASK_EXT 0x0100000000000000UL -#define PSW_MASK_DAT 0x0400000000000000UL -#define PSW_MASK_PSTATE 0x0001000000000000UL - #define CR0_EXTM_SCLP 0x0000000000000200UL #define CR0_EXTM_EXTC 0x0000000000002000UL #define CR0_EXTM_EMGC 0x0000000000004000UL @@ -297,4 +303,5 @@ static inline uint32_t get_prefix(void) return current_prefix; } +#endif /* __ASSEMBLER */ #endif diff --git a/s390x/cstart64.S b/s390x/cstart64.S index e084f13..6e85635 100644 --- a/s390x/cstart64.S +++ b/s390x/cstart64.S @@ -12,6 +12,7 @@ */ #include <asm/asm-offsets.h> #include <asm/sigp.h> +#include <asm/arch_def.h> .section .init @@ -198,19 +199,19 @@ svc_int: .align 8 reset_psw: - .quad 0x0008000180000000 + .quad PSW_EXCEPTION_MASK | PSW_MASK_SHORT_PSW initial_psw: - .quad 0x0000000180000000, clear_bss_start + .quad PSW_EXCEPTION_MASK, clear_bss_start pgm_int_psw: - .quad 0x0000000180000000, pgm_int + .quad PSW_EXCEPTION_MASK, pgm_int ext_int_psw: - .quad 0x0000000180000000, ext_int + .quad PSW_EXCEPTION_MASK, ext_int mcck_int_psw: - .quad 0x0000000180000000, mcck_int + .quad PSW_EXCEPTION_MASK, mcck_int io_int_psw: - .quad 0x0000000180000000, io_int + .quad PSW_EXCEPTION_MASK, io_int svc_int_psw: - .quad 0x0000000180000000, svc_int + .quad PSW_EXCEPTION_MASK, svc_int initial_cr0: /* enable AFP-register control, so FP regs (+BFP instr) can be used */ .quad 0x0000000000040000