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

[Richard Henderson]

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(-)
>
> diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
> index 3da337f7c7..b260bf7331 100644
> --- a/target/s390x/tcg/excp_helper.c
> +++ b/target/s390x/tcg/excp_helper.c
> @@ -190,11 +190,6 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int 
> size,
>           return false;
>       }
>   
> -    if (excp != PGM_ADDRESSING) {
> -        stq_phys(env_cpu(env)->as,
> -                 env->psa + offsetof(LowCore, trans_exc_code), tec);
> -    }
> -
>       /*
>        * For data accesses, ILEN will be filled in from the unwind info,
>        * within cpu_loop_exit_restore.  For code accesses, retaddr == 0,
> @@ -211,20 +206,34 @@ static void do_program_interrupt(CPUS390XState *env)
>       uint64_t mask, addr;
>       LowCore *lowcore;
>       int ilen = env->int_pgm_ilen;
> +    bool set_trans_exc_code = false;
> +    bool advance = false;
>   
>       assert((env->int_pgm_code == PGM_SPECIFICATION && ilen == 0) ||
>              ilen == 2 || ilen == 4 || ilen == 6);
>   
>       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.

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


r~