Re: [kvm-unit-tests PATCH v5 04/10] s390x: interrupt registration

Pierre Morel
2020-04-24T10:44:23Z

On 2020-04-24 10:27, Janosch Frank wrote:
> On 2/20/20 1:00 PM, Pierre Morel wrote:
>> Let's make it possible to add and remove a custom io interrupt handler,
>> that can be used instead of the normal one.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> lib/s390x/interrupt.c | 22 +++++++++++++++++++++-
>> lib/s390x/interrupt.h | 7 +++++++
>> 2 files changed, 28 insertions(+), 1 deletion(-)
>> create mode 100644 lib/s390x/interrupt.h
>>
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 3a40cac..f6f0665 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -10,9 +10,9 @@
>> * under the terms of the GNU Library General Public License version 2.
>> */
>> #include <libcflat.h>
>> -#include <asm/interrupt.h>
>> #include <asm/barrier.h>
>> #include <sclp.h>
>> +#include <interrupt.h>
>
> Hrm
>
>>
>> static bool pgm_int_expected;
>> static bool ext_int_expected;
>> @@ -144,12 +144,32 @@ void handle_mcck_int(void)
>> stap(), lc->mcck_old_psw.addr);
>> }
>>
>> +static void (*io_int_func)(void);
>> +
>> void handle_io_int(void)
>> {
>> + if (*io_int_func)
>> + return (*io_int_func)();
>> report_abort("Unexpected io interrupt: on cpu %d at %#lx",
>> stap(), lc->io_old_psw.addr);
>> }
>>
>> +int register_io_int_func(void (*f)(void))
>> +{
>> + if (io_int_func)
>> + return -1;
>> + io_int_func = f;
>> + return 0;
>> +}
>> +
>> +int unregister_io_int_func(void (*f)(void))
>> +{
>> + if (io_int_func != f)
>> + return -1;
>> + io_int_func = NULL;
>> + return 0;
>> +}
>
> I'm currently working on something similar for PGMs and I see no
> additional value in two functions for this. Unregistering can be done by
> doing register_io_int_func(NULL)
>
> This should be enough:
>
> int register_io_int_func(void (*f)(void))
> {
> io_int_func = f;
> }
>
There are several ways to do this and I really don't mind how it is done.
Since it has been reviewed by, I would like to have the others reviewers
opinion.

Regards,
Pierre

--
Pierre Morel
IBM Lab Boeblingen