On 7/23/20 2:01 PM, Cornelia Huck wrote:
> On Fri, 17 Jul 2020 10:58:11 -0400
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Sometimes we need to do cleanup which we don't necessarily want to add
>> to interrupt.c, so lets add a way to register a cleanup function.
> 
> s/lets/let's/ :)
> 
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  lib/s390x/asm/interrupt.h | 1 +
>>  lib/s390x/interrupt.c     | 9 +++++++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
>> index 4cfade9..b2a7c83 100644
>> --- a/lib/s390x/asm/interrupt.h
>> +++ b/lib/s390x/asm/interrupt.h
>> @@ -15,6 +15,7 @@
>>  #define EXT_IRQ_EXTERNAL_CALL	0x1202
>>  #define EXT_IRQ_SERVICE_SIG	0x2401
>>  
>> +void register_pgm_int_func(void (*f)(void));
>>  void handle_pgm_int(void);
>>  void handle_ext_int(void);
>>  void handle_mcck_int(void);
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 243b9c2..36ba720 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -16,6 +16,7 @@
>>  
>>  static bool pgm_int_expected;
>>  static bool ext_int_expected;
>> +static void (*pgm_int_func)(void);
>>  static struct lowcore *lc;
>>  
>>  void expect_pgm_int(void)
>> @@ -51,8 +52,16 @@ void check_pgm_int_code(uint16_t code)
>>  	       lc->pgm_int_code);
>>  }
>>  
>> +void register_pgm_int_func(void (*f)(void))
>> +{
>> +	pgm_int_func = f;
>> +}
>> +
>>  static void fixup_pgm_int(void)
>>  {
>> +	if (pgm_int_func)
>> +		return (*pgm_int_func)();
>> +
> 
> Maybe rather call this function, if set, instead of fixup_pgm_int() in
> handle_pgm_int()? Feels a bit cleaner to me.

Well it's currently a cleanup function so it should be in
fixup_pgm_int() because it fixes up.

I don't need a handler here like Pierre with his IO changes.

So it might more sense to change the name of the function ptr and
registration function:

register_pgm_cleanup_func()
static void (*pgm_cleanup_func)(void);

> 		
>>  	switch (lc->pgm_int_code) {
>>  	case PGM_INT_CODE_PRIVILEGED_OPERATION:
>>  		/* Normal operation is in supervisor state, so this exception
>