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

[Richard Henderson <richard.henderson@linaro.org>]

On 11/28/23 05:08, Philippe Mathieu-Daudé wrote:
> 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?

Probably not.

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

I suspect that should be migrated to use the common HW breakpoint support.


r~