On 26.09.19 18:26, Richard Henderson wrote:
> We currently set ilen to AUTO, then overwrite that during
> unwinding, then overwrite that for the code access case.
>
> This can be simplified to setting ilen to our arbitrary
> value for the (undefined) code access case, then rely on
> unwinding to overwrite that with the correct value for
> the data access case.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> target/s390x/excp_helper.c | 23 +++++++----------------
> 1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index 98a1ee8317..8ce992e639 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -96,7 +96,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int
> size,
> {
> S390CPU *cpu = S390_CPU(cs);
>
> - trigger_pgm_exception(&cpu->env, PGM_ADDRESSING, ILEN_AUTO);
> + trigger_pgm_exception(&cpu->env, PGM_ADDRESSING, ILEN_UNWIND);
Hmm, we always trigger a pgm exceptions, meaning we set
cs->exception_index even if we have probe = true. Confused by that.
> /* On real machines this value is dropped into LowMem. Since this
> is userland, simply put this someplace that cpu_loop can find it. */
> cpu->env.__excp_addr = address;
> @@ -179,24 +179,15 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int
> size,
> stq_phys(env_cpu(env)->as,
> env->psa + offsetof(LowCore, trans_exc_code), tec);
> }
> - trigger_pgm_exception(env, excp, ILEN_AUTO);
> - cpu_restore_state(cs, retaddr, true);
>
> /*
> - * The ILC value for code accesses is undefined. The important
> - * thing here is to *not* leave env->int_pgm_ilen set to ILEN_AUTO,
> - * which would cause do_program_interrupt to attempt to read from
> - * env->psw.addr again. C.f. the condition in trigger_page_fault,
> - * but is not universally applied.
> - *
> - * ??? If we remove ILEN_AUTO, by moving the computation of ILEN
> - * into cpu_restore_state, then we may remove this entirely.
> + * For data accesses, ILEN will be filled in from the unwind info,
> + * within cpu_loop_exit_restore. For code accesses, retaddr == 0,
> + * and so unwinding will not occur. However, ILEN is also undefined
> + * for that case -- we choose to set ILEN = 2.
> */
> - if (access_type == MMU_INST_FETCH) {
> - env->int_pgm_ilen = 2;
> - }
> -
> - cpu_loop_exit(cs);
> + trigger_pgm_exception(env, excp, 2);
I wonder if it is still worth setting this only conditionally. Most
probably not.
> + cpu_loop_exit_restore(cs, retaddr);
> }
>
> static void do_program_interrupt(CPUS390XState *env)
>
--
Thanks,
David / dhildenb
