diff mbox series

[kvm-unit-tests,v8,01/12] s390x: Use PSW bits definitions in cstart

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

Commit Message

Pierre Morel June 8, 2020, 8:12 a.m. UTC 
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(-)

Comments

Thomas Huth June 8, 2020, 8:43 a.m. UTC | #1 
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>
Pierre Morel June 8, 2020, 2:33 p.m. UTC | #2 
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
Thomas Huth June 8, 2020, 2:52 p.m. UTC | #3 
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
Pierre Morel June 8, 2020, 3:28 p.m. UTC | #4 
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
Thomas Huth June 8, 2020, 3:30 p.m. UTC | #5 
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 mbox series

Patch

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