Re: [PATCH] target/s390x: Move trans_exc_code update to do_program_interrupt

[Ilya Leoshkevich]

On Fri, 2023-07-28 at 13:02 -0700, Richard Henderson wrote:
> On 7/28/23 12:55, Richard Henderson wrote:
> > This solves a problem in which the store to LowCore during tlb_fill
> > triggers a clean-page TB invalidation for page0 during translation,
> > which results in an assertion failure for locked pages.
> > 
> > By delaying the store until after the exception has been raised,
> > we will have unwound the pages locked for translation and the
> > problem does not arise.  There are plenty of other updates to
> > LowCore while delivering an interrupt/exception; trans_exc_code
> > does not need to be special.
> > 
> > Reported-by: Claudio Fontana <cfontana@suse.de>
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   target/s390x/tcg/excp_helper.c | 42 +++++++++++++++++++++++------
> > -----
> >   1 file changed, 29 insertions(+), 13 deletions(-)

[...]

> >       switch (env->int_pgm_code) {
> >       case PGM_PER:
> > -        if (env->per_perc_atmid & PER_CODE_EVENT_NULLIFICATION) {
> > -            break;
> > -        }
> > -        /* FALL THROUGH */
> > +        advance = !(env->per_perc_atmid &
> > PER_CODE_EVENT_NULLIFICATION);
> > +        break;
> > +    case PGM_ASCE_TYPE:
> > +    case PGM_REG_FIRST_TRANS:
> > +    case PGM_REG_SEC_TRANS:
> > +    case PGM_REG_THIRD_TRANS:
> > +    case PGM_SEGMENT_TRANS:
> > +    case PGM_PAGE_TRANS:
> > +        assert(env->int_pgm_code == env->tlb_fill_exc);
> > +        set_trans_exc_code = true;
> > +        break;
> 
> I should have mentioned that this block of exceptions came from page
> 3-76 
> (Translation-Exception Identification for DAT Exceptions) of the 13th
> edition of the PoO.
> 
> > +    case PGM_PROTECTION:
> > +    case PGM_TRANS_SPEC:
> > +        assert(env->int_pgm_code == env->tlb_fill_exc);
> > +        set_trans_exc_code = true;
> > +        advance = true;
> > +        break;
> 
> These exceptions came from seeing an early kernel fault, grepping for
> the set of 
> exceptions raised in mmu_helper.c, and eliminating PGM_ADDRESSING per
> the first hunk.

Does POp specify that the CPU stores Translation-Exception
Identification on Translation-Specification Exceptions
(PGM_TRANS_SPEC)? I re-read the 0xA8 documentation a few times, but
could not find it.

It's also interesting what the kernel was attempting when it got
PGM_TRANS_SPEC and recovered from it. Maybe something else is wrong?

Other than the POp question:

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

> 
> I wasn't sure where to look for the full specification of exception
> effects, but this did 
> solve the kernel fault.
> 
> 
> r~