On Tue, 7 Jun 2022 12:01:11 +0200
Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> wrote:
> On 6/3/22 17:40, Claudio Imbrenda wrote:
> > Use per-CPU flags and callbacks for Program, Extern, and I/O interrupts
> > instead of global variables.
> >
> > This allows for more accurate error handling; a CPU waiting for an
> > interrupt will not have it "stolen" by a different CPU that was not
> > supposed to wait for one, and now two CPUs can wait for interrupts at
> > the same time.
> >
> > Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> > ---
> > lib/s390x/asm/arch_def.h | 7 ++++++-
> > lib/s390x/interrupt.c | 38 ++++++++++++++++----------------------
> > 2 files changed, 22 insertions(+), 23 deletions(-)
> >
> > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> > index 72553819..3a0d9c43 100644
> > --- a/lib/s390x/asm/arch_def.h
> > +++ b/lib/s390x/asm/arch_def.h
> > @@ -124,7 +124,12 @@ struct lowcore {
> > uint8_t pad_0x0280[0x0308 - 0x0280]; /* 0x0280 */
> > uint64_t sw_int_crs[16]; /* 0x0308 */
> > struct psw sw_int_psw; /* 0x0388 */
> > - uint8_t pad_0x0310[0x11b0 - 0x0398]; /* 0x0398 */
> > + uint32_t pgm_int_expected; /* 0x0398 */
> > + uint32_t ext_int_expected; /* 0x039c */
> > + void (*pgm_cleanup_func)(void); /* 0x03a0 */
> > + void (*ext_cleanup_func)(void); /* 0x03a8 */
> > + void (*io_int_func)(void); /* 0x03b0 */
>
> If you switch the function pointers and the *_expected around,
> you can use bools for the latter, right?
> I think, since they're names suggest that they're bools, they should
> be. Additionally I prefer true/false over 1/0, since the latter raises
> the questions if other values are also used.
that's exactly what I wanted to avoid. uint32_t can easily be accessed
atomically and/or compare-and-swapped if needed.
I don't like using true/false for things that are not bools
>
> > + uint8_t pad_0x03b8[0x11b0 - 0x03b8]; /* 0x03b8 */
> > uint64_t mcck_ext_sa_addr; /* 0x11b0 */
> > uint8_t pad_0x11b8[0x1200 - 0x11b8]; /* 0x11b8 */
> > uint64_t fprs_sa[16]; /* 0x1200 */
> > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> > index 27d3b767..e57946f0 100644
> > --- a/lib/s390x/interrupt.c
> > +++ b/lib/s390x/interrupt.c
> > @@ -15,14 +15,11 @@
> > #include <fault.h>
> > #include <asm/page.h>
> >
> > -static bool pgm_int_expected;
> > -static bool ext_int_expected;
> > -static void (*pgm_cleanup_func)(void);
> > static struct lowcore *lc;
> >
> > void expect_pgm_int(void)
> > {
> > - pgm_int_expected = true;
> > + lc->pgm_int_expected = 1;
> > lc->pgm_int_code = 0;
> > lc->trans_exc_id = 0;
> > mb();
>
> [...]
>
> > void handle_pgm_int(struct stack_frame_int *stack)
> > {
> > - if (!pgm_int_expected) {
> > + if (!lc->pgm_int_expected) {
> > /* Force sclp_busy to false, otherwise we will loop forever */
> > sclp_handle_ext();
> > print_pgm_info(stack);
> > }
> >
> > - pgm_int_expected = false;
> > + lc->pgm_int_expected = 0;
> >
> > - if (pgm_cleanup_func)
> > - (*pgm_cleanup_func)();
> > + if (lc->pgm_cleanup_func)
> > + (*lc->pgm_cleanup_func)();
>
> [...]
>
> > + if (lc->io_int_func)
> > + return lc->io_int_func();
> Why is a difference between the function pointer usages here?
>
because that is how it was before; both have the same semantics anyway
