Re: [PATCH 1/8] accel: Replace `target_ulong` with `vaddr` in TB/TLB

[Richard Henderson]

On 4/20/23 22:28, Anton Johansson wrote:
> Changes pc and cs_base in TranslationBlock from target_ulong to vaddr.
> Auxilliary structs and tb_*()/tlb_*() functions that depend on this
> change are also updated to take a vaddr for guest virtual addresses.
>
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   accel/stubs/tcg-stub.c       |   2 +-
>   accel/tcg/cpu-exec.c         |  49 +++++-----
>   accel/tcg/cputlb.c           | 179 +++++++++++++++++------------------
>   accel/tcg/internal.h         |   6 +-
>   accel/tcg/tb-hash.h          |  12 +--
>   accel/tcg/tb-jmp-cache.h     |   2 +-
>   accel/tcg/tb-maint.c         |   2 +-
>   accel/tcg/translate-all.c    |  15 +--
>   include/exec/cpu-defs.h      |   4 +-
>   include/exec/cpu_ldst.h      |   6 +-
>   include/exec/exec-all.h      |  82 ++++++++--------
>   include/qemu/plugin-memory.h |   2 +-
>   12 files changed, 181 insertions(+), 180 deletions(-)

This is too large and must be split.  In addition, there are places where you must take 
more care with the replacement.

> @@ -412,10 +412,11 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
>   {
>       CPUState *cpu = env_cpu(env);
>       TranslationBlock *tb;
> -    target_ulong cs_base, pc;
> +    vaddr cs_base = 0, pc = 0;
>       uint32_t flags, cflags;
>   
> -    cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
> +    cpu_get_tb_cpu_state(env, (target_ulong *) &pc,
> +                         (target_ulong *) &cs_base, &flags);

This simply will not work on big-endian hosts.

> @@ -543,7 +544,8 @@ void cpu_exec_step_atomic(CPUState *cpu)
>           g_assert(!cpu->running);
>           cpu->running = true;
>   
> -        cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
> +        cpu_get_tb_cpu_state(env, (target_ulong *) &pc,
> +                             (target_ulong *) &cs_base, &flags);

Likewise.

> @@ -946,10 +948,11 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
>   
>           while (!cpu_handle_interrupt(cpu, &last_tb)) {
>               TranslationBlock *tb;
> -            target_ulong cs_base, pc;
> +            vaddr cs_base = 0, pc = 0;
>               uint32_t flags, cflags;
>   
> -            cpu_get_tb_cpu_state(cpu->env_ptr, &pc, &cs_base, &flags);
> +            cpu_get_tb_cpu_state(cpu->env_ptr, (target_ulong *) &pc,
> +                                 (target_ulong *) &cs_base, &flags);

Likewise.  And one more in translate-all.c.

> @@ -560,15 +557,15 @@ static void tlb_flush_page_by_mmuidx_async_0(CPUState 
> *cpu,
>   static void tlb_flush_page_by_mmuidx_async_1(CPUState *cpu,
>                                                run_on_cpu_data data)
>   {
> -    target_ulong addr_and_idxmap = (target_ulong) data.target_ptr;
> -    target_ulong addr = addr_and_idxmap & TARGET_PAGE_MASK;
> +    vaddr addr_and_idxmap = (vaddr) data.target_ptr;

run_on_cpu_data.target_ptr is already vaddr, no need for cast.


r~