Re: [PATCH for-6.1 v6 14/17] accel/tcg: Move breakpoint recognition outside translation

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi,

On 20/7/21 21:54, Richard Henderson wrote:
> Trigger breakpoints before beginning translation of a TB
> that would begin with a BP.  Thus we never generate code
> for the BP at all.
> 
> Single-step instructions within a page containing a BP so
> that we are sure to check each insn for the BP as above.
> 
> We no longer need to flush any TBs when changing BPs.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/286
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/404
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/489
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/cpu-exec.c   | 91 ++++++++++++++++++++++++++++++++++++++++--
>   accel/tcg/translator.c | 24 +----------
>   cpu.c                  | 20 ----------
>   3 files changed, 89 insertions(+), 46 deletions(-)


> diff --git a/cpu.c b/cpu.c
> index 91d9e38acb..d6ae5ae581 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -225,11 +225,6 @@ void tb_invalidate_phys_addr(target_ulong addr)
>       tb_invalidate_phys_page_range(addr, addr + 1);
>       mmap_unlock();
>   }
> -
> -static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
> -{
> -    tb_invalidate_phys_addr(pc);
> -}

This patch removed the last use of tb_invalidate_phys_addr() in
user emulation:

   void tb_invalidate_phys_addr(hwaddr addr)
   {
       mmap_lock();
       tb_invalidate_phys_page(addr);
       mmap_unlock();
   }

Do we still need it?

(In sysemu there is a single use in Xtensa tb_invalidate_virtual_addr).

>   #else
>   void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs)
>   {
> @@ -250,17 +245,6 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs)
>       ram_addr = memory_region_get_ram_addr(mr) + addr;
>       tb_invalidate_phys_page_range(ram_addr, ram_addr + 1);
>   }
> -
> -static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
> -{
> -    /*
> -     * There may not be a virtual to physical translation for the pc
> -     * right now, but there may exist cached TB for this pc.
> -     * Flush the whole TB cache to force re-translation of such TBs.
> -     * This is heavyweight, but we're debugging anyway.
> -     */
> -    tb_flush(cpu);
> -}
>   #endif