diff mbox series

[kvm-unit-tests,v2,2/9] s390x: Define the PSW bits

Message ID 1574945167-29677-3-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 Nov. 28, 2019, 12:46 p.m. UTC
Let's define the PSW bits  explicitly, it will clarify their
usage.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/asm/arch_bits.h | 20 ++++++++++++++++++++
 lib/s390x/asm/arch_def.h  |  6 ++----
 s390x/cstart64.S          | 13 +++++++------
 3 files changed, 29 insertions(+), 10 deletions(-)
 create mode 100644 lib/s390x/asm/arch_bits.h

Comments

David Hildenbrand Nov. 28, 2019, 2:36 p.m. UTC | #1
On 28.11.19 13:46, Pierre Morel wrote:
> Let's define the PSW bits  explicitly, it will clarify their
> usage.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_bits.h | 20 ++++++++++++++++++++
>  lib/s390x/asm/arch_def.h  |  6 ++----

I'm sorry, but I don't really see a reason to move these 4/5 defines to
a separate header. Can you just keep them in arch_def.h and extend?

(none of your other patches touch arch_bits.h - and it is somewhat a
weird name. Where to put something new: arch_def.h or arch_bits.h? I
would have understood "psw.h", but even that, I don't consider necessary)
Pierre Morel Nov. 28, 2019, 7:16 p.m. UTC | #2
On 2019-11-28 15:36, David Hildenbrand wrote:
> On 28.11.19 13:46, Pierre Morel wrote:
>> Let's define the PSW bits  explicitly, it will clarify their
>> usage.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/asm/arch_bits.h | 20 ++++++++++++++++++++
>>   lib/s390x/asm/arch_def.h  |  6 ++----
> I'm sorry, but I don't really see a reason to move these 4/5 defines to
> a separate header. Can you just keep them in arch_def.h and extend?

no because arch_def.h contains C structures and inline.


>
> (none of your other patches touch arch_bits.h - and it is somewhat a
> weird name. Where to put something new: arch_def.h or arch_bits.h? I
> would have understood "psw.h", but even that, I don't consider necessary)
>
I can use a name like psw.h or let fall and keep the hexa.
David Hildenbrand Nov. 28, 2019, 7:48 p.m. UTC | #3
> Am 28.11.2019 um 20:16 schrieb Pierre Morel <pmorel@linux.ibm.com>:
> 
> 
>> On 2019-11-28 15:36, David Hildenbrand wrote:
>>> On 28.11.19 13:46, Pierre Morel wrote:
>>> Let's define the PSW bits  explicitly, it will clarify their
>>> usage.
>>> 
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>  lib/s390x/asm/arch_bits.h | 20 ++++++++++++++++++++
>>>  lib/s390x/asm/arch_def.h  |  6 ++----
>> I'm sorry, but I don't really see a reason to move these 4/5 defines to
>> a separate header. Can you just keep them in arch_def.h and extend?
> 
> no because arch_def.h contains C structures and inline.

(resend because the iOS Mail app does crazy html thingies)

You‘re looking for

#ifndef __ASSEMBLER__
...

See lib/s390x/asm/sigp.h
Pierre Morel Nov. 29, 2019, 1:04 p.m. UTC | #4
On 2019-11-28 20:48, David Hildenbrand wrote:
> 
> 
>> Am 28.11.2019 um 20:16 schrieb Pierre Morel <pmorel@linux.ibm.com>:
>>
>> 
>>> On 2019-11-28 15:36, David Hildenbrand wrote:
>>>> On 28.11.19 13:46, Pierre Morel wrote:
>>>> Let's define the PSW bits  explicitly, it will clarify their
>>>> usage.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>   lib/s390x/asm/arch_bits.h | 20 ++++++++++++++++++++
>>>>   lib/s390x/asm/arch_def.h  |  6 ++----
>>> I'm sorry, but I don't really see a reason to move these 4/5 defines to
>>> a separate header. Can you just keep them in arch_def.h and extend?
>>
>> no because arch_def.h contains C structures and inline.
> 
> (resend because the iOS Mail app does crazy html thingies)
> 
> You‘re looking for
> 
> #ifndef __ASSEMBLER__
> ...
> 
> See lib/s390x/asm/sigp.h
> 

Yes, better. :)

Thanks,
Pierre
Janosch Frank Dec. 2, 2019, 11:11 a.m. UTC | #5
On 11/28/19 3:36 PM, David Hildenbrand wrote:
> On 28.11.19 13:46, Pierre Morel wrote:
>> Let's define the PSW bits  explicitly, it will clarify their
>> usage.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>  lib/s390x/asm/arch_bits.h | 20 ++++++++++++++++++++
>>  lib/s390x/asm/arch_def.h  |  6 ++----
> 
> I'm sorry, but I don't really see a reason to move these 4/5 defines to
> a separate header. Can you just keep them in arch_def.h and extend?
> 
> (none of your other patches touch arch_bits.h - and it is somewhat a
> weird name. Where to put something new: arch_def.h or arch_bits.h? I
> would have understood "psw.h", but even that, I don't consider necessary)
> 

On a related note:
I'd still like to split up the file soonish, maybe moving the functions
into a new file?

@Thomas/David: What's your opinion on that?
David Hildenbrand Dec. 2, 2019, 11:17 a.m. UTC | #6
On 02.12.19 12:11, Janosch Frank wrote:
> On 11/28/19 3:36 PM, David Hildenbrand wrote:
>> On 28.11.19 13:46, Pierre Morel wrote:
>>> Let's define the PSW bits  explicitly, it will clarify their
>>> usage.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>  lib/s390x/asm/arch_bits.h | 20 ++++++++++++++++++++
>>>  lib/s390x/asm/arch_def.h  |  6 ++----
>>
>> I'm sorry, but I don't really see a reason to move these 4/5 defines to
>> a separate header. Can you just keep them in arch_def.h and extend?
>>
>> (none of your other patches touch arch_bits.h - and it is somewhat a
>> weird name. Where to put something new: arch_def.h or arch_bits.h? I
>> would have understood "psw.h", but even that, I don't consider necessary)
>>
> 
> On a related note:
> I'd still like to split up the file soonish, maybe moving the functions
> into a new file?
> 
> @Thomas/David: What's your opinion on that?

Sure, as long as the file header is not longer as its actual content :D
Pierre Morel Dec. 2, 2019, 4:52 p.m. UTC | #7
On 2019-12-02 12:17, David Hildenbrand wrote:
> On 02.12.19 12:11, Janosch Frank wrote:
>> On 11/28/19 3:36 PM, David Hildenbrand wrote:
>>> On 28.11.19 13:46, Pierre Morel wrote:
>>>> Let's define the PSW bits  explicitly, it will clarify their
>>>> usage.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>   lib/s390x/asm/arch_bits.h | 20 ++++++++++++++++++++
>>>>   lib/s390x/asm/arch_def.h  |  6 ++----
>>>
>>> I'm sorry, but I don't really see a reason to move these 4/5 defines to
>>> a separate header. Can you just keep them in arch_def.h and extend?
>>>
>>> (none of your other patches touch arch_bits.h - and it is somewhat a
>>> weird name. Where to put something new: arch_def.h or arch_bits.h? I
>>> would have understood "psw.h", but even that, I don't consider necessary)
>>>
>>
>> On a related note:
>> I'd still like to split up the file soonish, maybe moving the functions
>> into a new file?
>>
>> @Thomas/David: What's your opinion on that?
> 
> Sure, as long as the file header is not longer as its actual content :D
> 

No problem for me.
What should the file header be?
diff mbox series

Patch

diff --git a/lib/s390x/asm/arch_bits.h b/lib/s390x/asm/arch_bits.h
new file mode 100644
index 0000000..a77489a
--- /dev/null
+++ b/lib/s390x/asm/arch_bits.h
@@ -0,0 +1,20 @@ 
+/*
+ * Copyright (c) 2019 IBM Corp
+ *
+ * Authors:
+ *  Pierre Morel <pmorel@linux.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2.
+ */
+#ifndef _ASM_S390X_ARCH_BITS_H_
+#define _ASM_S390X_ARCH_BITS_H_
+
+#define PSW_MASK_IO		0x0200000000000000
+#define PSW_MASK_EXT		0x0100000000000000
+#define PSW_MASK_EA		0x0000000100000000
+#define PSW_MASK_BA		0x0000000080000000
+
+#define PSW_EXCEPTION_MASK (PSW_MASK_EA|PSW_MASK_BA)
+
+#endif
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index cf6e1ca..ee7ace2 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -10,15 +10,13 @@ 
 #ifndef _ASM_S390X_ARCH_DEF_H_
 #define _ASM_S390X_ARCH_DEF_H_
 
+#include <asm/arch_bits.h>
+
 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
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 525c464..44caf7a 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -11,6 +11,7 @@ 
  * under the terms of the GNU Library General Public License version 2.
  */
 #include <asm/asm-offsets.h>
+#include <asm/arch_bits.h>
 #include <asm/sigp.h>
 
 .section .init
@@ -216,17 +217,17 @@  svc_int:
 reset_psw:
 	.quad	0x0008000180000000
 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