Re: [PULL 15/57] target/mips: Pass ptw_mmu_idx down from mips_cpu_tlb_fill

[Richard Henderson]

On 8/10/24 04:11, Philippe Mathieu-Daudé wrote:
> Hi Richard,
>
> On 2/2/24 06:49, Richard Henderson wrote:
> > Rather than adjust env->hflags so that the value computed
> > by cpu_mmu_index() changes, compute the mmu_idx that we
> > want directly and pass it down.
> >
> > Introduce symbolic constants for MMU_{KERNEL,ERL}_IDX.
> >
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   target/mips/cpu.h                   |  4 +++-
> >   target/mips/tcg/sysemu/tlb_helper.c | 32 ++++++++++++-----------------
> >   2 files changed, 16 insertions(+), 20 deletions(-)
>
>
> > @@ -944,12 +940,10 @@ bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int 
> > size,
> >            * Memory reads during hardware page table walking are performed
> >            * as if they were kernel-mode load instructions.
> >            */
> > -        int mode = (env->hflags & MIPS_HFLAG_KSU);
> > -        bool ret_walker;
> > -        env->hflags &= ~MIPS_HFLAG_KSU;
> > -        ret_walker = page_table_walk_refill(env, address, mmu_idx);
> > -        env->hflags |= mode;
> > -        if (ret_walker) {
> > +        int ptw_mmu_idx = (env->hflags & MIPS_HFLAG_ERL ?
> > +                           MMU_ERL_IDX : MMU_KERNEL_IDX);
>
> Checking https://gitlab.com/qemu-project/qemu/-/issues/2470.
>
> Parenthesis are mis-placed.
>
>            int ptw_mmu_idx = (env->hflags & MIPS_HFLAG_ERL) ?
>                               MMU_ERL_IDX : MMU_KERNEL_IDX;

This makes no difference to the evaluation of this expression.

> Revisiting, we loose possible MMU_USER_IDX value but
> - we don't use it
> - this is sysemu code so we only expect MMU_KERNEL_IDX
>
> Is that right?

The comment above is correct that ptw reads are performed in kernel mode.

The code previously saved the current mode, cleared the user bit, performed the operation, 
and then restored the previous mode.  There was no possible MMU_USER_IDX during that interval.

The code currently skips the save/restore and simply selects MMU_KERNEL_IDX.


r~