diff mbox series

[kvm-unit-tests,v1,2/3] s390x: check for specific program interrupt

Message ID 20210209185154.1037852-3-imbrenda@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: mvpg test | expand

Commit Message

Claudio Imbrenda Feb. 9, 2021, 6:51 p.m. UTC
We already have check_pgm_int_code to check and report if a specific
program interrupt has occourred, but this approach has some issues.

In order to specify which test is being run, it was needed to push and
pop a prefix for each test, which is not nice to read both in the code
and in the output.

Another issue is that sometimes the condition to test for might require
other checks in addition to the interrupt.

The simple function added in this patch tests if the program intteruupt
received is the one expected.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/asm/interrupt.h | 1 +
 lib/s390x/interrupt.c     | 6 ++++++
 2 files changed, 7 insertions(+)

Comments

Cornelia Huck Feb. 10, 2021, 11:23 a.m. UTC | #1
On Tue,  9 Feb 2021 19:51:53 +0100
Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:

> We already have check_pgm_int_code to check and report if a specific
> program interrupt has occourred, but this approach has some issues.

s/occourred/occurred/

> 
> In order to specify which test is being run, it was needed to push and
> pop a prefix for each test, which is not nice to read both in the code
> and in the output.
> 
> Another issue is that sometimes the condition to test for might require
> other checks in addition to the interrupt.
> 
> The simple function added in this patch tests if the program intteruupt

s/intteruupt/interrupt/

> received is the one expected.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>  lib/s390x/asm/interrupt.h | 1 +
>  lib/s390x/interrupt.c     | 6 ++++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
> index 1a2e2cd8..a33437b1 100644
> --- a/lib/s390x/asm/interrupt.h
> +++ b/lib/s390x/asm/interrupt.h
> @@ -23,6 +23,7 @@ void expect_pgm_int(void);
>  void expect_ext_int(void);
>  uint16_t clear_pgm_int(void);
>  void check_pgm_int_code(uint16_t code);
> +int is_pgm(int expected);
>  
>  /* Activate low-address protection */
>  static inline void low_prot_enable(void)
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 59e01b1a..6f660285 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -51,6 +51,12 @@ void check_pgm_int_code(uint16_t code)
>  	       lc->pgm_int_code);
>  }
>  
> +int is_pgm(int expected)

is_pgm() is a bit non-descriptive. Maybe check_pgm_int_code_noreport()?

Also, maybe let it take a uint16_t parameter?

> +{
> +	mb();
> +	return expected == lc->pgm_int_code;
> +}
> +
>  void register_pgm_cleanup_func(void (*f)(void))
>  {
>  	pgm_cleanup_func = f;
Janosch Frank Feb. 10, 2021, 11:43 a.m. UTC | #2
On 2/10/21 12:23 PM, Cornelia Huck wrote:
> On Tue,  9 Feb 2021 19:51:53 +0100
> Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
> 
>> We already have check_pgm_int_code to check and report if a specific
>> program interrupt has occourred, but this approach has some issues.
> 
> s/occourred/occurred/
> 
>>
>> In order to specify which test is being run, it was needed to push and
>> pop a prefix for each test, which is not nice to read both in the code
>> and in the output.
>>
>> Another issue is that sometimes the condition to test for might require
>> other checks in addition to the interrupt.
>>
>> The simple function added in this patch tests if the program intteruupt
> 
> s/intteruupt/interrupt/
> 
>> received is the one expected.
>>
>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>>  lib/s390x/asm/interrupt.h | 1 +
>>  lib/s390x/interrupt.c     | 6 ++++++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
>> index 1a2e2cd8..a33437b1 100644
>> --- a/lib/s390x/asm/interrupt.h
>> +++ b/lib/s390x/asm/interrupt.h
>> @@ -23,6 +23,7 @@ void expect_pgm_int(void);
>>  void expect_ext_int(void);
>>  uint16_t clear_pgm_int(void);
>>  void check_pgm_int_code(uint16_t code);
>> +int is_pgm(int expected);
>>  
>>  /* Activate low-address protection */
>>  static inline void low_prot_enable(void)
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 59e01b1a..6f660285 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -51,6 +51,12 @@ void check_pgm_int_code(uint16_t code)
>>  	       lc->pgm_int_code);
>>  }
>>  
>> +int is_pgm(int expected)
> 
> is_pgm() is a bit non-descriptive. Maybe check_pgm_int_code_noreport()?
> 
> Also, maybe let it take a uint16_t parameter?

Could we use clear_pgm_int()?
It returns the last code so you can check yourself.

We could rename it to fetch_and_clear_pgm_int()

> 
>> +{
>> +	mb();
>> +	return expected == lc->pgm_int_code;
>> +}
>> +
>>  void register_pgm_cleanup_func(void (*f)(void))
>>  {
>>  	pgm_cleanup_func = f;
>
diff mbox series

Patch

diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
index 1a2e2cd8..a33437b1 100644
--- a/lib/s390x/asm/interrupt.h
+++ b/lib/s390x/asm/interrupt.h
@@ -23,6 +23,7 @@  void expect_pgm_int(void);
 void expect_ext_int(void);
 uint16_t clear_pgm_int(void);
 void check_pgm_int_code(uint16_t code);
+int is_pgm(int expected);
 
 /* Activate low-address protection */
 static inline void low_prot_enable(void)
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 59e01b1a..6f660285 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -51,6 +51,12 @@  void check_pgm_int_code(uint16_t code)
 	       lc->pgm_int_code);
 }
 
+int is_pgm(int expected)
+{
+	mb();
+	return expected == lc->pgm_int_code;
+}
+
 void register_pgm_cleanup_func(void (*f)(void))
 {
 	pgm_cleanup_func = f;