[kvm-unit-tests,v7,5/8] s390x: lib: sie: don't reenter SIE on pgm int

Commit Message

Nico Boehr Nov. 3, 2023, 9:29 a.m. UTC

At the moment, when a PGM int occurs while in SIE, we will just reenter
SIE after the interrupt handler was called.

This is because sie() has a loop which checks icptcode and re-enters SIE
if it is zero.

However, this behaviour is quite undesirable for SIE tests, since it
doesn't give the host the chance to assert on the PGM int. Instead, we
will just re-enter SIE, on nullifing conditions even causing the
exception again.

In sie(), check whether a pgm int code is set in lowcore. If it has,
exit the loop so the test can react to the interrupt. Add a new function
read_pgm_int_code() to obtain the interrupt code.

Note that this introduces a slight oddity with sie and pgm int in
certain cases: If a PGM int occurs between a expect_pgm_int() and sie(),
we will now never enter SIE until the pgm_int_code is cleared by e.g.
clear_pgm_int().

Also add missing include of facility.h to mem.h.

Signed-off-by: Nico Boehr <[e-mail]>
Reviewed-by: Thomas Huth <[e-mail]>
---
 lib/s390x/asm/interrupt.h | 14 ++++++++++++++
 lib/s390x/asm/mem.h       |  1 +
 lib/s390x/sie.c           |  4 +++-
 3 files changed, 18 insertions(+), 1 deletion(-)

Comments

Claudio Imbrenda Nov. 3, 2023, 1:48 p.m. UTC | #1

On Fri,  3 Nov 2023 10:29:34 +0100
Nico Boehr <[e-mail]> wrote:

[...]

> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
> index d01f8a89641a..7f73d473b346 100644
> --- a/lib/s390x/asm/interrupt.h
> +++ b/lib/s390x/asm/interrupt.h
> @@ -97,4 +97,18 @@ static inline void low_prot_disable(void)
>  	ctl_clear_bit(0, CTL0_LOW_ADDR_PROT);
>  }
>  
> +/**
> + * read_pgm_int_code - Get the program interruption code of the last pgm int
> + * on the current CPU.
> + *
> + * This is similar to clear_pgm_int(), except that it doesn't clear the
> + * interruption information from lowcore.
> + *
> + * Returns 0 when none occurred.

 * Return:

with that fixed:

Reviewed-by: Claudio Imbrenda <[e-mail]>

[...]

Janosch Frank Nov. 3, 2023, 1:53 p.m. UTC | #2

On 11/3/23 10:29, Nico Boehr wrote:
> At the moment, when a PGM int occurs while in SIE, we will just reenter
> SIE after the interrupt handler was called.
> 
> This is because sie() has a loop which checks icptcode and re-enters SIE
> if it is zero.
> 
> However, this behaviour is quite undesirable for SIE tests, since it
> doesn't give the host the chance to assert on the PGM int. Instead, we
> will just re-enter SIE, on nullifing conditions even causing the
> exception again.
> 
> In sie(), check whether a pgm int code is set in lowcore. If it has,
> exit the loop so the test can react to the interrupt. Add a new function
> read_pgm_int_code() to obtain the interrupt code.
> 
> Note that this introduces a slight oddity with sie and pgm int in
> certain cases: If a PGM int occurs between a expect_pgm_int() and sie(),
> we will now never enter SIE until the pgm_int_code is cleared by e.g.
> clear_pgm_int().

Is there any use in NOT having an assert(!read_pgm_int_code()) before 
entering the loop?

Nico Boehr Nov. 6, 2023, 4:36 p.m. UTC | #3

Quoting Janosch Frank (2023-11-03 14:53:17)
> On 11/3/23 10:29, Nico Boehr wrote:
> > At the moment, when a PGM int occurs while in SIE, we will just reenter
> > SIE after the interrupt handler was called.
> > 
> > This is because sie() has a loop which checks icptcode and re-enters SIE
> > if it is zero.
> > 
> > However, this behaviour is quite undesirable for SIE tests, since it
> > doesn't give the host the chance to assert on the PGM int. Instead, we
> > will just re-enter SIE, on nullifing conditions even causing the
> > exception again.
> > 
> > In sie(), check whether a pgm int code is set in lowcore. If it has,
> > exit the loop so the test can react to the interrupt. Add a new function
> > read_pgm_int_code() to obtain the interrupt code.
> > 
> > Note that this introduces a slight oddity with sie and pgm int in
> > certain cases: If a PGM int occurs between a expect_pgm_int() and sie(),
> > we will now never enter SIE until the pgm_int_code is cleared by e.g.
> > clear_pgm_int().
> 
> Is there any use in NOT having an assert(!read_pgm_int_code()) before 
> entering the loop?

I added it, nothing breaks, so probably none. Thanks.

Patch

diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
index d01f8a89641a..7f73d473b346 100644
--- a/lib/s390x/asm/interrupt.h
+++ b/lib/s390x/asm/interrupt.h
@@ -97,4 +97,18 @@  static inline void low_prot_disable(void)
 	ctl_clear_bit(0, CTL0_LOW_ADDR_PROT);
 }
 
+/**
+ * read_pgm_int_code - Get the program interruption code of the last pgm int
+ * on the current CPU.
+ *
+ * This is similar to clear_pgm_int(), except that it doesn't clear the
+ * interruption information from lowcore.
+ *
+ * Returns 0 when none occurred.
+ */
+static inline uint16_t read_pgm_int_code(void)
+{
+	return lowcore.pgm_int_code;
+}
+
 #endif
diff --git a/lib/s390x/asm/mem.h b/lib/s390x/asm/mem.h
index 64ef59b546a4..94d58c34f53f 100644
--- a/lib/s390x/asm/mem.h
+++ b/lib/s390x/asm/mem.h
@@ -8,6 +8,7 @@ 
 #ifndef _ASMS390X_MEM_H_
 #define _ASMS390X_MEM_H_
 #include <asm/arch_def.h>
+#include <asm/facility.h>
 
 /* create pointer while avoiding compiler warnings */
 #define OPAQUE_PTR(x) ((void *)(((uint64_t)&lowcore) + (x)))
diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
index 7f4474555ff7..a97096948460 100644
--- a/lib/s390x/sie.c
+++ b/lib/s390x/sie.c
@@ -13,6 +13,7 @@ 
 #include <libcflat.h>
 #include <sie.h>
 #include <asm/page.h>
+#include <asm/interrupt.h>
 #include <libcflat.h>
 #include <alloc_page.h>
 
@@ -79,7 +80,8 @@  void sie(struct vm *vm)
 	/* also handle all interruptions in home space while in SIE */
 	irq_set_dat_mode(true, AS_HOME);
 
-	while (vm->sblk->icptcode == 0) {
+	/* leave SIE when we have an intercept or an interrupt so the test can react to it */
+	while (vm->sblk->icptcode == 0 && !read_pgm_int_code()) {
 		sie64a(vm->sblk, &vm->save_area);
 		sie_handle_validity(vm);
 	}