Quoting Janosch Frank (2023-06-05 11:30:36)
> On 6/1/23 09:02, Nico Boehr wrote:
> > At the moment, when a PGM int occurs while in SIE, we will just reenter
> > SIE after the interrupt handler was called.
> >
> > This is because sie() has a loop which checks icptcode and re-enters SIE
> > if it is zero.
> >
> > However, this behaviour is quite undesirable for SIE tests, since it
> > doesn't give the host the chance to assert on the PGM int. Instead, we
> > will just re-enter SIE, on nullifing conditions even causing the
> > exception again.
>
> That's the reason why we set an invalid PGM PSW new for the assembly
> snippets. Seems like I didn't add it for C snippets for some reason -_-
True, C snippets should have a invalid PGM new PSW too. Let me have a try after
my holiday... *writes TODO*
> This code is fine but it doesn't fully fix the usability aspect and
> leaves a few questions open:
> - Do we want to stick to the code 8 handling?
Well, I think we need to distinguish between two kinds of PGMs:
- PGMs in the guest,
- PGMs caused by SIE on the host (e.g. because the gpa-hpa mapping is not
present)
The first case is out of scope for this patch, but certainly something which can
be improved.
This patch focuses on the latter case, where code 8 handling is irrelevant since
the PGM is always delivered to the host.
> - Do we want to assert like with validities and PGMs outside of SIE?
Well, we would assert() except if we have an expect_pgm_int(), which we do have
:)
> - Should sie() have a int return code like in KVM?
That's a lovely idea, maybe we should have this at some point in the future, but
I suppose it would require reshuffling large parts of the tests, so please
excuse me if I don't want to do this in the context of this series :) *writes
another TODO*
[...]
> > Also add missing include of facility.h to mem.h.
>
> ?
That explains why we're including facility.h in mem.h below.
[...]
> > diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
> > index 55759002dce2..fb4283a40a1b 100644
> > --- a/lib/s390x/asm/interrupt.h
> > +++ b/lib/s390x/asm/interrupt.h
> > @@ -99,4 +99,18 @@ static inline void low_prot_disable(void)
> > ctl_clear_bit(0, CTL0_LOW_ADDR_PROT);
> > }
> >
> > +/**
> > + * read_pgm_int_code - Get the program interruption code of the last pgm int
> > + * on the current CPU.
>
> All of the other functions are in the c file.
Claudio requested this to be in the C file, I really don't mind much. Claudio,
maybe you can elaborate why you wanted it in the header.
> > + *
> > + * This is similar to clear_pgm_int(), except that it doesn't clear the
> > + * interruption information from lowcore.
> > + *
> > + * Returns 0 when none occured.
>
> s/r/rr/
Fixed.
> > + */
> > +static inline uint16_t read_pgm_int_code(void)
> > +{
>
> No mb()?
This is a function call, so none should be needed, no?
