diff mbox

[kvm-unit-tests,v2] s390x: Catch all exceptions

Message ID 20180627143152.121933-1-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Janosch Frank June 27, 2018, 2:31 p.m. UTC 
Right now we only catch the exceptions that we really expect to
receive. Let's at least catch all of them and abort if any of the
unexpected ones fell on our foot.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---

I hope I understood you correctly, as assembly is not my strongest
language skill.

---
 lib/s390x/interrupt.c | 24 +++++++++++++++++++++
 s390x/cstart64.S      | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 79 insertions(+), 3 deletions(-)

Comments

Thomas Huth June 27, 2018, 3:36 p.m. UTC | #1 
On 27.06.2018 16:31, Janosch Frank wrote:
> Right now we only catch the exceptions that we really expect to
> receive. Let's at least catch all of them and abort if any of the
> unexpected ones fell on our foot.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> 
> I hope I understood you correctly, as assembly is not my strongest
> language skill.
> 
> ---
>  lib/s390x/interrupt.c | 24 +++++++++++++++++++++
>  s390x/cstart64.S      | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 79 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index bc44e3a..848d5f2 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -104,3 +104,27 @@ void handle_pgm_int(void)
>  	pgm_int_expected = false;
>  	fixup_pgm_int();
>  }
> +
> +void handle_ext_int(void)
> +{
> +	report_abort("Unexpected external call interrupt: at %#lx",
> +		     lc->ext_old_psw.addr);
> +}
> +
> +void handle_mcck_int(void)
> +{
> +	report_abort("Unexpected machine check interrupt: at %#lx",
> +		     lc->mcck_old_psw.addr);
> +}
> +
> +void handle_io_int(void)
> +{
> +	report_abort("Unexpected io interrupt: at %#lx",
> +		     lc->io_old_psw.addr);
> +}
> +
> +void handle_svc_int(void)
> +{
> +	report_abort("Unexpected service call interrupt: at %#lx",
> +		     lc->svc_old_psw.addr);
> +}
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 9a26ed3..3ead2d2 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -26,6 +26,18 @@ init_psw_cont:
>  	/* setup pgm interrupt handler */
>  	larl	%r1, pgm_int_psw
>  	mvc	GEN_LC_PGM_NEW_PSW(16), 0(%r1)
> +	/* setup ext interrupt handler */
> +	larl	%r1, ext_int_psw
> +	mvc	GEN_LC_EXT_NEW_PSW(16), 0(%r1)
> +	/* setup mcck interrupt handler */
> +	larl	%r1, mcck_int_psw
> +	mvc	GEN_LC_MCCK_NEW_PSW(16), 0(%r1)
> +	/* setup io interrupt handler */
> +	larl	%r1, ext_int_psw
> +	mvc	GEN_LC_IO_NEW_PSW(16), 0(%r1)
> +	/* setup svc interrupt handler */
> +	larl	%r1, mcck_int_psw
> +	mvc	GEN_LC_SVC_NEW_PSW(16), 0(%r1)
>  	/* setup cr0, enabling e.g. AFP-register control */
>  	larl	%r1, initital_cr0
>  	lctlg	%c0, %c0, 0(%r1)
> @@ -42,7 +54,7 @@ init_psw_cont:
>  	/* call exit() */
>  	j exit
>  
> -pgm_int:
> +.macro SAVE_REGS
>  	/* save grs 0-15 */
>  	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
>  	/* save fprs 0-15 + fpc */
> @@ -64,8 +76,9 @@ pgm_int:
>  	std	%f14, 112(%r1)
>  	std	%f15, 120(%r1)
>  	stfpc	GEN_LC_SW_INT_FPC
> -	/* call our c handler */
> -	brasl	%r14, handle_pgm_int
> +.endm
> +
> +.macro RESTORE_REGS
>  	/* restore fprs 0-15 + fpc */
>  	larl	%r1, GEN_LC_SW_INT_FPRS
>  	ld	%f0, 0(%r1)
> @@ -87,13 +100,52 @@ pgm_int:
>  	lfpc	GEN_LC_SW_INT_FPC
>  	/* restore grs 0-15 */
>  	lmg	%r0, %r15, GEN_LC_SW_INT_GRS
> +.endm
> +
> +.section .text
> +pgm_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_pgm_int
> +	RESTORE_REGS
>  	lpswe	GEN_LC_PGM_OLD_PSW
>  
> +ext_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_ext_int
> +	RESTORE_REGS
> +	lpswe	GEN_LC_EXT_OLD_PSW
> +
> +mcck_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_mcck_int
> +	RESTORE_REGS
> +	lpswe	GEN_LC_MCCK_OLD_PSW
> +
> +io_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_io_int
> +	RESTORE_REGS
> +	lpswe	GEN_LC_IO_OLD_PSW
> +
> +svc_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_svc_int
> +	RESTORE_REGS
> +	lpswe	GEN_LC_SVC_OLD_PSW
> +
>  	.align	8
>  initital_psw:
>  	.quad	0x0000000180000000, init_psw_cont
>  pgm_int_psw:
>  	.quad	0x0000000180000000, pgm_int
> +ext_int_psw:
> +	.quad	0x0000000180000000, ext_int
> +mcck_int_psw:
> +	.quad	0x0000000180000000, mcck_int
> +io_int_psw:
> +	.quad	0x0000000180000000, io_int
> +svc_int_psw:
> +	.quad	0x0000000180000000, svc_int
>  initital_cr0:
>  	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
>  	.quad	0x0000000000040000

Reviewed-by: Thomas Huth <thuth@redhat.com>
David Hildenbrand June 28, 2018, 8:11 a.m. UTC | #2 
On 27.06.2018 16:31, Janosch Frank wrote:
> Right now we only catch the exceptions that we really expect to
> receive. Let's at least catch all of them and abort if any of the
> unexpected ones fell on our foot.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> 
> I hope I understood you correctly, as assembly is not my strongest
> language skill.
> 
> ---
>  lib/s390x/interrupt.c | 24 +++++++++++++++++++++
>  s390x/cstart64.S      | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 79 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index bc44e3a..848d5f2 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -104,3 +104,27 @@ void handle_pgm_int(void)
>  	pgm_int_expected = false;
>  	fixup_pgm_int();
>  }
> +
> +void handle_ext_int(void)
> +{
> +	report_abort("Unexpected external call interrupt: at %#lx",
> +		     lc->ext_old_psw.addr);
> +}
> +
> +void handle_mcck_int(void)
> +{
> +	report_abort("Unexpected machine check interrupt: at %#lx",
> +		     lc->mcck_old_psw.addr);
> +}
> +
> +void handle_io_int(void)
> +{
> +	report_abort("Unexpected io interrupt: at %#lx",
> +		     lc->io_old_psw.addr);
> +}
> +
> +void handle_svc_int(void)
> +{
> +	report_abort("Unexpected service call interrupt: at %#lx",
> +		     lc->svc_old_psw.addr);
> +}

We need prototypes for these in lib/s390x/asm/interrupt.h if I'm not wrong.

> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 9a26ed3..3ead2d2 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -26,6 +26,18 @@ init_psw_cont:
>  	/* setup pgm interrupt handler */
>  	larl	%r1, pgm_int_psw
>  	mvc	GEN_LC_PGM_NEW_PSW(16), 0(%r1)
> +	/* setup ext interrupt handler */
> +	larl	%r1, ext_int_psw
> +	mvc	GEN_LC_EXT_NEW_PSW(16), 0(%r1)
> +	/* setup mcck interrupt handler */
> +	larl	%r1, mcck_int_psw
> +	mvc	GEN_LC_MCCK_NEW_PSW(16), 0(%r1)
> +	/* setup io interrupt handler */
> +	larl	%r1, ext_int_psw
> +	mvc	GEN_LC_IO_NEW_PSW(16), 0(%r1)
> +	/* setup svc interrupt handler */
> +	larl	%r1, mcck_int_psw
> +	mvc	GEN_LC_SVC_NEW_PSW(16), 0(%r1)
>  	/* setup cr0, enabling e.g. AFP-register control */
>  	larl	%r1, initital_cr0
>  	lctlg	%c0, %c0, 0(%r1)
> @@ -42,7 +54,7 @@ init_psw_cont:
>  	/* call exit() */
>  	j exit
>  
> -pgm_int:
> +.macro SAVE_REGS

I remember that macros are also intended (looking at e.g.
arch/s390/kernel/entry.S)

>  	/* save grs 0-15 */
>  	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
>  	/* save fprs 0-15 + fpc */
> @@ -64,8 +76,9 @@ pgm_int:
>  	std	%f14, 112(%r1)
>  	std	%f15, 120(%r1)
>  	stfpc	GEN_LC_SW_INT_FPC
> -	/* call our c handler */
> -	brasl	%r14, handle_pgm_int
> +.endm
> +
> +.macro RESTORE_REGS
>  	/* restore fprs 0-15 + fpc */
>  	larl	%r1, GEN_LC_SW_INT_FPRS
>  	ld	%f0, 0(%r1)
> @@ -87,13 +100,52 @@ pgm_int:
>  	lfpc	GEN_LC_SW_INT_FPC
>  	/* restore grs 0-15 */
>  	lmg	%r0, %r15, GEN_LC_SW_INT_GRS
> +.endm
> +
> +.section .text
> +pgm_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_pgm_int
> +	RESTORE_REGS
>  	lpswe	GEN_LC_PGM_OLD_PSW
>  
> +ext_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_ext_int
> +	RESTORE_REGS
> +	lpswe	GEN_LC_EXT_OLD_PSW
> +
> +mcck_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_mcck_int
> +	RESTORE_REGS
> +	lpswe	GEN_LC_MCCK_OLD_PSW
> +
> +io_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_io_int
> +	RESTORE_REGS
> +	lpswe	GEN_LC_IO_OLD_PSW
> +
> +svc_int:
> +	SAVE_REGS
> +	brasl	%r14, handle_svc_int
> +	RESTORE_REGS
> +	lpswe	GEN_LC_SVC_OLD_PSW
> +
>  	.align	8
>  initital_psw:
>  	.quad	0x0000000180000000, init_psw_cont
>  pgm_int_psw:
>  	.quad	0x0000000180000000, pgm_int
> +ext_int_psw:
> +	.quad	0x0000000180000000, ext_int
> +mcck_int_psw:
> +	.quad	0x0000000180000000, mcck_int
> +io_int_psw:
> +	.quad	0x0000000180000000, io_int
> +svc_int_psw:
> +	.quad	0x0000000180000000, svc_int
>  initital_cr0:
>  	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
>  	.quad	0x0000000000040000
> 

Apart from that, looks good to me.
Thomas Huth July 10, 2018, 6:44 a.m. UTC | #3 
On 28.06.2018 10:11, David Hildenbrand wrote:
> On 27.06.2018 16:31, Janosch Frank wrote:
>> Right now we only catch the exceptions that we really expect to
>> receive. Let's at least catch all of them and abort if any of the
>> unexpected ones fell on our foot.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>
>> I hope I understood you correctly, as assembly is not my strongest
>> language skill.
>>
>> ---
>>  lib/s390x/interrupt.c | 24 +++++++++++++++++++++
>>  s390x/cstart64.S      | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 79 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index bc44e3a..848d5f2 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -104,3 +104,27 @@ void handle_pgm_int(void)
>>  	pgm_int_expected = false;
>>  	fixup_pgm_int();
>>  }
>> +
>> +void handle_ext_int(void)
>> +{
>> +	report_abort("Unexpected external call interrupt: at %#lx",
>> +		     lc->ext_old_psw.addr);
>> +}
>> +
>> +void handle_mcck_int(void)
>> +{
>> +	report_abort("Unexpected machine check interrupt: at %#lx",
>> +		     lc->mcck_old_psw.addr);
>> +}
>> +
>> +void handle_io_int(void)
>> +{
>> +	report_abort("Unexpected io interrupt: at %#lx",
>> +		     lc->io_old_psw.addr);
>> +}
>> +
>> +void handle_svc_int(void)
>> +{
>> +	report_abort("Unexpected service call interrupt: at %#lx",
>> +		     lc->svc_old_psw.addr);
>> +}
> 
> We need prototypes for these in lib/s390x/asm/interrupt.h if I'm not wrong.

We can also add them later, when we finally really turn on the
-Wmissing-prototypes compiler flag.

I think the patch is good to go already. Paolo, Radim, could you please
pick up this patch directly? I don't have any other patches pending ...
or do you still want me to rather send a PULL request for it?

 Thomas
Thomas Huth July 10, 2018, 6:46 a.m. UTC | #4 
On 10.07.2018 08:44, Thomas Huth wrote:
> On 28.06.2018 10:11, David Hildenbrand wrote:
>> On 27.06.2018 16:31, Janosch Frank wrote:
>>> Right now we only catch the exceptions that we really expect to
>>> receive. Let's at least catch all of them and abort if any of the
>>> unexpected ones fell on our foot.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>
>>> I hope I understood you correctly, as assembly is not my strongest
>>> language skill.
>>>
>>> ---
>>>  lib/s390x/interrupt.c | 24 +++++++++++++++++++++
>>>  s390x/cstart64.S      | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
>>>  2 files changed, 79 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>>> index bc44e3a..848d5f2 100644
>>> --- a/lib/s390x/interrupt.c
>>> +++ b/lib/s390x/interrupt.c
>>> @@ -104,3 +104,27 @@ void handle_pgm_int(void)
>>>  	pgm_int_expected = false;
>>>  	fixup_pgm_int();
>>>  }
>>> +
>>> +void handle_ext_int(void)
>>> +{
>>> +	report_abort("Unexpected external call interrupt: at %#lx",
>>> +		     lc->ext_old_psw.addr);
>>> +}
>>> +
>>> +void handle_mcck_int(void)
>>> +{
>>> +	report_abort("Unexpected machine check interrupt: at %#lx",
>>> +		     lc->mcck_old_psw.addr);
>>> +}
>>> +
>>> +void handle_io_int(void)
>>> +{
>>> +	report_abort("Unexpected io interrupt: at %#lx",
>>> +		     lc->io_old_psw.addr);
>>> +}
>>> +
>>> +void handle_svc_int(void)
>>> +{
>>> +	report_abort("Unexpected service call interrupt: at %#lx",
>>> +		     lc->svc_old_psw.addr);
>>> +}
>>
>> We need prototypes for these in lib/s390x/asm/interrupt.h if I'm not wrong.
> 
> We can also add them later, when we finally really turn on the
> -Wmissing-prototypes compiler flag.
> 
> I think the patch is good to go already. Paolo, Radim, could you please
> pick up this patch directly? I don't have any other patches pending ...
> or do you still want me to rather send a PULL request for it?

Ah, never mind, sorry, I missed v3 of this patch.

 Thomas
diff mbox

Patch

diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index bc44e3a..848d5f2 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -104,3 +104,27 @@  void handle_pgm_int(void)
 	pgm_int_expected = false;
 	fixup_pgm_int();
 }
+
+void handle_ext_int(void)
+{
+	report_abort("Unexpected external call interrupt: at %#lx",
+		     lc->ext_old_psw.addr);
+}
+
+void handle_mcck_int(void)
+{
+	report_abort("Unexpected machine check interrupt: at %#lx",
+		     lc->mcck_old_psw.addr);
+}
+
+void handle_io_int(void)
+{
+	report_abort("Unexpected io interrupt: at %#lx",
+		     lc->io_old_psw.addr);
+}
+
+void handle_svc_int(void)
+{
+	report_abort("Unexpected service call interrupt: at %#lx",
+		     lc->svc_old_psw.addr);
+}
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 9a26ed3..3ead2d2 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -26,6 +26,18 @@  init_psw_cont:
 	/* setup pgm interrupt handler */
 	larl	%r1, pgm_int_psw
 	mvc	GEN_LC_PGM_NEW_PSW(16), 0(%r1)
+	/* setup ext interrupt handler */
+	larl	%r1, ext_int_psw
+	mvc	GEN_LC_EXT_NEW_PSW(16), 0(%r1)
+	/* setup mcck interrupt handler */
+	larl	%r1, mcck_int_psw
+	mvc	GEN_LC_MCCK_NEW_PSW(16), 0(%r1)
+	/* setup io interrupt handler */
+	larl	%r1, ext_int_psw
+	mvc	GEN_LC_IO_NEW_PSW(16), 0(%r1)
+	/* setup svc interrupt handler */
+	larl	%r1, mcck_int_psw
+	mvc	GEN_LC_SVC_NEW_PSW(16), 0(%r1)
 	/* setup cr0, enabling e.g. AFP-register control */
 	larl	%r1, initital_cr0
 	lctlg	%c0, %c0, 0(%r1)
@@ -42,7 +54,7 @@  init_psw_cont:
 	/* call exit() */
 	j exit
 
-pgm_int:
+.macro SAVE_REGS
 	/* save grs 0-15 */
 	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
 	/* save fprs 0-15 + fpc */
@@ -64,8 +76,9 @@  pgm_int:
 	std	%f14, 112(%r1)
 	std	%f15, 120(%r1)
 	stfpc	GEN_LC_SW_INT_FPC
-	/* call our c handler */
-	brasl	%r14, handle_pgm_int
+.endm
+
+.macro RESTORE_REGS
 	/* restore fprs 0-15 + fpc */
 	larl	%r1, GEN_LC_SW_INT_FPRS
 	ld	%f0, 0(%r1)
@@ -87,13 +100,52 @@  pgm_int:
 	lfpc	GEN_LC_SW_INT_FPC
 	/* restore grs 0-15 */
 	lmg	%r0, %r15, GEN_LC_SW_INT_GRS
+.endm
+
+.section .text
+pgm_int:
+	SAVE_REGS
+	brasl	%r14, handle_pgm_int
+	RESTORE_REGS
 	lpswe	GEN_LC_PGM_OLD_PSW
 
+ext_int:
+	SAVE_REGS
+	brasl	%r14, handle_ext_int
+	RESTORE_REGS
+	lpswe	GEN_LC_EXT_OLD_PSW
+
+mcck_int:
+	SAVE_REGS
+	brasl	%r14, handle_mcck_int
+	RESTORE_REGS
+	lpswe	GEN_LC_MCCK_OLD_PSW
+
+io_int:
+	SAVE_REGS
+	brasl	%r14, handle_io_int
+	RESTORE_REGS
+	lpswe	GEN_LC_IO_OLD_PSW
+
+svc_int:
+	SAVE_REGS
+	brasl	%r14, handle_svc_int
+	RESTORE_REGS
+	lpswe	GEN_LC_SVC_OLD_PSW
+
 	.align	8
 initital_psw:
 	.quad	0x0000000180000000, init_psw_cont
 pgm_int_psw:
 	.quad	0x0000000180000000, pgm_int
+ext_int_psw:
+	.quad	0x0000000180000000, ext_int
+mcck_int_psw:
+	.quad	0x0000000180000000, mcck_int
+io_int_psw:
+	.quad	0x0000000180000000, io_int
+svc_int_psw:
+	.quad	0x0000000180000000, svc_int
 initital_cr0:
 	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
 	.quad	0x0000000000040000