[kvm-unit-tests PATCH v1 0/2] s390x misc fixes

[kvm-unit-tests PATCH v1 0/2] s390x misc fixes

[Claudio Imbrenda]

Two unrelated misc fixes for s390x:

* abort immediately and without printing anything if a program interrupt is
  received inside an interrupt handler
* fix the uv-host test so that it allocates UV memory properly

Claudio Imbrenda (2):
  lib: s390x: terminate if PGM interrupt in interrupt handler
  s390x: uv-host: fix allocation of UV memory

 lib/s390x/asm/arch_def.h | 11 +++++++++++
 lib/s390x/interrupt.c    | 18 ++++++++++++++----
 s390x/uv-host.c          |  2 +-
 3 files changed, 26 insertions(+), 5 deletions(-)

-- 
2.37.3

[kvm-unit-tests PATCH v1 1/2] lib: s390x: terminate if PGM interrupt in interrupt handler

[Claudio Imbrenda]

If a program interrupt is received while in an interrupt handler,
terminate immediately, stopping all CPUs and leaving the last CPU in
disabled wait with a specific PSW code.

This will aid debugging by not cluttering the output, avoiding further
interrupts (that would be needed to write to the output), and providing
an indication of the cause of the termination.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h | 11 +++++++++++
 lib/s390x/interrupt.c    | 18 ++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index b92291e8..124449a8 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -51,6 +51,7 @@ struct cpu {
 	bool active;
 	bool pgm_int_expected;
 	bool ext_int_expected;
+	bool in_interrupt_handler;
 };
 
 #define AS_PRIM				0
@@ -330,6 +331,16 @@ static inline void load_psw_mask(uint64_t mask)
 		: "+r" (tmp) :  "a" (&psw) : "memory", "cc" );
 }
 
+static inline void disabled_wait(uint64_t message)
+{
+	struct psw psw = {
+		.mask = PSW_MASK_WAIT,  /* Disabled wait */
+		.addr = message,
+	};
+
+	asm volatile("  lpswe 0(%0)\n" : : "a" (&psw) : "memory", "cc");
+}
+
 /**
  * psw_mask_clear_bits - clears bits from the current PSW mask
  * @clear: bitmask of bits that will be cleared
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 7cc2c5fb..22bf443b 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -14,6 +14,7 @@
 #include <sie.h>
 #include <fault.h>
 #include <asm/page.h>
+#include "smp.h"
 
 /**
  * expect_pgm_int - Expect a program interrupt on the current CPU.
@@ -226,6 +227,11 @@ static void print_pgm_info(struct stack_frame_int *stack)
 
 void handle_pgm_int(struct stack_frame_int *stack)
 {
+	if (THIS_CPU->in_interrupt_handler) {
+		/* Something went very wrong, stop everything now without printing anything */
+		smp_teardown();
+		disabled_wait(0xfa12edbad21);
+	}
 	if (!THIS_CPU->pgm_int_expected) {
 		/* Force sclp_busy to false, otherwise we will loop forever */
 		sclp_handle_ext();
@@ -242,6 +248,7 @@ void handle_pgm_int(struct stack_frame_int *stack)
 
 void handle_ext_int(struct stack_frame_int *stack)
 {
+	THIS_CPU->in_interrupt_handler = true;
 	if (!THIS_CPU->ext_int_expected && lowcore.ext_int_code != EXT_IRQ_SERVICE_SIG) {
 		report_abort("Unexpected external call interrupt (code %#x): on cpu %d at %#lx",
 			     lowcore.ext_int_code, stap(), lowcore.ext_old_psw.addr);
@@ -260,6 +267,7 @@ void handle_ext_int(struct stack_frame_int *stack)
 
 	if (THIS_CPU->ext_cleanup_func)
 		THIS_CPU->ext_cleanup_func(stack);
+	THIS_CPU->in_interrupt_handler = false;
 }
 
 void handle_mcck_int(void)
@@ -272,11 +280,13 @@ static void (*io_int_func)(void);
 
 void handle_io_int(void)
 {
+	THIS_CPU->in_interrupt_handler = true;
 	if (io_int_func)
-		return io_int_func();
-
-	report_abort("Unexpected io interrupt: on cpu %d at %#lx",
-		     stap(), lowcore.io_old_psw.addr);
+		io_int_func();
+	else
+		report_abort("Unexpected io interrupt: on cpu %d at %#lx",
+			     stap(), lowcore.io_old_psw.addr);
+	THIS_CPU->in_interrupt_handler = false;
 }
 
 int register_io_int_func(void (*f)(void))
-- 
2.37.3

Re: [kvm-unit-tests PATCH v1 1/2] lib: s390x: terminate if PGM interrupt in interrupt handler

[Nico Boehr]

Quoting Claudio Imbrenda (2022-10-18 16:09:50)
[...]
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 7cc2c5fb..22bf443b 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
[...]
>  void handle_pgm_int(struct stack_frame_int *stack)
>  {
> +       if (THIS_CPU->in_interrupt_handler) {
> +               /* Something went very wrong, stop everything now without printing anything */
> +               smp_teardown();
> +               disabled_wait(0xfa12edbad21);
> +       }

Maybe I am missing something, but is there a particular reson why you don't do
 THIS_CPU->in_interrupt_handler = true;
here as well?

Re: [kvm-unit-tests PATCH v1 1/2] lib: s390x: terminate if PGM interrupt in interrupt handler

[Claudio Imbrenda]

On Wed, 19 Oct 2022 09:34:26 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> Quoting Claudio Imbrenda (2022-10-18 16:09:50)
> [...]
> > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> > index 7cc2c5fb..22bf443b 100644
> > --- a/lib/s390x/interrupt.c
> > +++ b/lib/s390x/interrupt.c  
> [...]
> >  void handle_pgm_int(struct stack_frame_int *stack)
> >  {
> > +       if (THIS_CPU->in_interrupt_handler) {
> > +               /* Something went very wrong, stop everything now without printing anything */
> > +               smp_teardown();
> > +               disabled_wait(0xfa12edbad21);
> > +       }  
> 
> Maybe I am missing something, but is there a particular reson why you don't do
>  THIS_CPU->in_interrupt_handler = true;
> here as well?

I was thinking that we set pgm_int_expected = false so we would catch a
wild program interrupt there, but in hindsight maybe it's better to set
in_interrupt_handler = true there so we can abort immediately

Re: [kvm-unit-tests PATCH v1 1/2] lib: s390x: terminate if PGM interrupt in interrupt handler

[Nico Boehr]

Quoting Claudio Imbrenda (2022-10-19 11:51:28)
[...]
> I was thinking that we set pgm_int_expected = false so we would catch a
> wild program interrupt there, but in hindsight maybe it's better to set
> in_interrupt_handler = true there so we can abort immediately

Oh right I missed that. I think how it is right now is nicer because we will get a nice message on the console, right?

In this case:
Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

Re: [kvm-unit-tests PATCH v1 1/2] lib: s390x: terminate if PGM interrupt in interrupt handler

[Claudio Imbrenda]

On Thu, 20 Oct 2022 09:58:05 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> Quoting Claudio Imbrenda (2022-10-19 11:51:28)
> [...]
> > I was thinking that we set pgm_int_expected = false so we would catch a
> > wild program interrupt there, but in hindsight maybe it's better to set
> > in_interrupt_handler = true there so we can abort immediately  
> 
> Oh right I missed that. I think how it is right now is nicer because we will get a nice message on the console, right?

which will generate more interrupts

@Janosch do you think it's better with or without setting
in_interrupt_handler in the pgm interrupt handler?

> 
> In this case:
> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

Re: [kvm-unit-tests PATCH v1 1/2] lib: s390x: terminate if PGM interrupt in interrupt handler

[Janosch Frank]

On 10/20/22 10:57, Claudio Imbrenda wrote:
> On Thu, 20 Oct 2022 09:58:05 +0200
> Nico Boehr <nrb@linux.ibm.com> wrote:
> 
>> Quoting Claudio Imbrenda (2022-10-19 11:51:28)
>> [...]
>>> I was thinking that we set pgm_int_expected = false so we would catch a
>>> wild program interrupt there, but in hindsight maybe it's better to set
>>> in_interrupt_handler = true there so we can abort immediately
>>
>> Oh right I missed that. I think how it is right now is nicer because we will get a nice message on the console, right?
> 
> which will generate more interrupts
> 
> @Janosch do you think it's better with or without setting
> in_interrupt_handler in the pgm interrupt handler?
> 

Any reason why you didn't set it in CALL_INT_HANDLER?

>>
>> In this case:
>> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
> 

Re: [kvm-unit-tests PATCH v1 1/2] lib: s390x: terminate if PGM interrupt in interrupt handler

[Claudio Imbrenda]

On Thu, 20 Oct 2022 13:19:36 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 10/20/22 10:57, Claudio Imbrenda wrote:
> > On Thu, 20 Oct 2022 09:58:05 +0200
> > Nico Boehr <nrb@linux.ibm.com> wrote:
> >   
> >> Quoting Claudio Imbrenda (2022-10-19 11:51:28)
> >> [...]  
> >>> I was thinking that we set pgm_int_expected = false so we would catch a
> >>> wild program interrupt there, but in hindsight maybe it's better to set
> >>> in_interrupt_handler = true there so we can abort immediately  
> >>
> >> Oh right I missed that. I think how it is right now is nicer because we will get a nice message on the console, right?  
> > 
> > which will generate more interrupts
> > 
> > @Janosch do you think it's better with or without setting
> > in_interrupt_handler in the pgm interrupt handler?
> >   
> 
> Any reason why you didn't set it in CALL_INT_HANDLER?

because then it will always be set whenever we get a PGM, the if will
always be true

> 
> >>
> >> In this case:
> >> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>  
> >   

Re: [kvm-unit-tests PATCH v1 1/2] lib: s390x: terminate if PGM interrupt in interrupt handler

[Janosch Frank]

On 10/20/22 13:45, Claudio Imbrenda wrote:
> On Thu, 20 Oct 2022 13:19:36 +0200
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 10/20/22 10:57, Claudio Imbrenda wrote:
>>> On Thu, 20 Oct 2022 09:58:05 +0200
>>> Nico Boehr <nrb@linux.ibm.com> wrote:
>>>    
>>>> Quoting Claudio Imbrenda (2022-10-19 11:51:28)
>>>> [...]
>>>>> I was thinking that we set pgm_int_expected = false so we would catch a
>>>>> wild program interrupt there, but in hindsight maybe it's better to set
>>>>> in_interrupt_handler = true there so we can abort immediately
>>>>
>>>> Oh right I missed that. I think how it is right now is nicer because we will get a nice message on the console, right?
>>>
>>> which will generate more interrupts
>>>
>>> @Janosch do you think it's better with or without setting
>>> in_interrupt_handler in the pgm interrupt handler?
>>>    
>>
>> Any reason why you didn't set it in CALL_INT_HANDLER?
> 
> because then it will always be set whenever we get a PGM, the if will
> always be true

Alright, then let's do in_interrupt_handler = true

> 
>>
>>>>
>>>> In this case:
>>>> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
>>>    
> 

[kvm-unit-tests PATCH v1 2/2] s390x: uv-host: fix allocation of UV memory

[Claudio Imbrenda]

Allocate the donated storage with 1M alignment from the normal pool, to
force it to be above 2G without wasting a whole 2G block of memory.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 s390x/uv-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index a1a6d120..e1fc0213 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -329,7 +329,7 @@ static void test_init(void)
 	struct psw psw;
 
 	/* Donated storage needs to be over 2GB */
-	mem = (uint64_t)memalign(1UL << 31, uvcb_qui.uv_base_stor_len);
+	mem = (uint64_t)memalign_pages_flags(SZ_1M, uvcb_qui.uv_base_stor_len, AREA_NORMAL);
 
 	uvcb_init.header.len = sizeof(uvcb_init);
 	uvcb_init.header.cmd = UVC_CMD_INIT_UV;
-- 
2.37.3

Re: [kvm-unit-tests PATCH v1 2/2] s390x: uv-host: fix allocation of UV memory

[Janosch Frank]

On 10/18/22 16:09, Claudio Imbrenda wrote:
> Allocate the donated storage with 1M alignment from the normal pool, to
> force it to be above 2G without wasting a whole 2G block of memory.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Thanks for fixing this :)
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>   s390x/uv-host.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index a1a6d120..e1fc0213 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -329,7 +329,7 @@ static void test_init(void)
>   	struct psw psw;
>   
>   	/* Donated storage needs to be over 2GB */
> -	mem = (uint64_t)memalign(1UL << 31, uvcb_qui.uv_base_stor_len);
> +	mem = (uint64_t)memalign_pages_flags(SZ_1M, uvcb_qui.uv_base_stor_len, AREA_NORMAL);
>   
>   	uvcb_init.header.len = sizeof(uvcb_init);
>   	uvcb_init.header.cmd = UVC_CMD_INIT_UV;

Re: [kvm-unit-tests PATCH v1 2/2] s390x: uv-host: fix allocation of UV memory

[Steffen Eiden]

On 10/18/22 16:09, Claudio Imbrenda wrote:
> Allocate the donated storage with 1M alignment from the normal pool, to
> force it to be above 2G without wasting a whole 2G block of memory.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
LGTM
Reviewed-by; Steffen Eiden <seiden@linux.ibm.com>

> ---
>   s390x/uv-host.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index a1a6d120..e1fc0213 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -329,7 +329,7 @@ static void test_init(void)
>   	struct psw psw;
>   
>   	/* Donated storage needs to be over 2GB */
> -	mem = (uint64_t)memalign(1UL << 31, uvcb_qui.uv_base_stor_len);
> +	mem = (uint64_t)memalign_pages_flags(SZ_1M, uvcb_qui.uv_base_stor_len, AREA_NORMAL);
>   
>   	uvcb_init.header.len = sizeof(uvcb_init);
>   	uvcb_init.header.cmd = UVC_CMD_INIT_UV;