Re: [PATCH v4 9/9] target/arm: Enable TARGET_TB_PCREL

[Richard Henderson]

On 9/22/22 07:07, Peter Maydell wrote:
> > +    /*
> > +     * For TARGET_TB_PCREL, the value relative to pc_curr against which
> > +     * offsets must be computed for cpu_pc.  -1 if unknown due to jump.
> > +     */
>
> I'm not really sure what this comment is trying to tell me. Could
> you expand it a bit ?

After the effect of an indirect jump is applied, pc_save is set to -1, so that we can 
assert that we don't attempt to use cpu_pc again.

> >   void arm_cpu_synchronize_from_tb(CPUState *cs,
> >                                    const TranslationBlock *tb)
> >   {
> > -    ARMCPU *cpu = ARM_CPU(cs);
> > -    CPUARMState *env = &cpu->env;
> > -
> > -    /*
> > -     * It's OK to look at env for the current mode here, because it's
> > -     * never possible for an AArch64 TB to chain to an AArch32 TB.
> > -     */
> > -    if (is_a64(env)) {
> > -        env->pc = tb_pc(tb);
> > -    } else {
> > -        env->regs[15] = tb_pc(tb);
> > +    /* The program counter is always up to date with TARGET_TB_PCREL. */
>
> Is it?

At TranslationBlock boundaries, yes.

> Is there some documentation in one of the other patchsets about
> how TARGET_TB_PCREL works in general and what targets need to do to
> support it?

I tried to do so in the generic TARGET_TB_PCREL support patch.

(1) PC must be up-to-date on all TB boundaries,
(2) translation is given a full virtual address, for the benefit
    of the (nested) call to cpu_ld*_code, but should otherwise
    only consider the page offset portion of that address.
    (a) unwind info is a page offset,
        (1) restore_state_to_opc must use a deposit, and
        (2) arch_tr_insn_start must mask the translation pc.
    (b) pass a complete virtual address to translator_use_goto_tb,
        which will compare that to the translation pc to decide if
        we've crossed a page boundary.
(3) Relative updates to PC should use runtime rather than
    translation time arithmetic.


r~