| Message ID | 1582200043-21760-5-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Testing the Channel Subsystem I/O | expand |
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; } > + > void handle_svc_int(void) > { > report_abort("Unexpected supervisor call interrupt: on cpu %d at %#lx", > diff --git a/lib/s390x/interrupt.h b/lib/s390x/interrupt.h > new file mode 100644 > index 0000000..e945ef7 > --- /dev/null > +++ b/lib/s390x/interrupt.h > @@ -0,0 +1,7 @@ > +#ifndef __INTERRUPT_H > +#include <asm/interrupt.h> > + > +int register_io_int_func(void (*f)(void)); > +int unregister_io_int_func(void (*f)(void)); > + > +#endif >
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
On Fri, 24 Apr 2020 12:44:16 +0200 Pierre Morel <pmorel@linux.ibm.com> wrote: > 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 > >> > >> +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; > > } You can even make this void :) > > > 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. One version might make it easier to catch programming errors, while the other one is more compact. I don't really have a preference on this, either is fine with me.
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> 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; +} + void handle_svc_int(void) { report_abort("Unexpected supervisor call interrupt: on cpu %d at %#lx", diff --git a/lib/s390x/interrupt.h b/lib/s390x/interrupt.h new file mode 100644 index 0000000..e945ef7 --- /dev/null +++ b/lib/s390x/interrupt.h @@ -0,0 +1,7 @@ +#ifndef __INTERRUPT_H +#include <asm/interrupt.h> + +int register_io_int_func(void (*f)(void)); +int unregister_io_int_func(void (*f)(void)); + +#endif