On 11/27/19 10:06 PM, Max Filippov wrote:
> When a breakpoint is inserted at location for which there's currently no
> virtual to physical translation no action is taken on CPU TB cache. If a
> TB for that virtual address already exists but is not visible ATM the
> breakpoint won't be hit next time an instruction at that address will be
> executed.
>
> Flush entire CPU TB cache in breakpoint_invalidate to force
> re-translation of all TBs for the breakpoint address.
>
> This change fixes the following scenario:
> - linux user application is running
> - a breakpoint is inserted from QEMU gdbstub for a user address that is
> not currently present in the target CPU TLB
> - an instruction at that address is executed, but the external debugger
> doesn't get control.
>
> Signed-off-by: Max Filippov <jcmvb...@gmail.com>
> ---
> Changes RFC->v1:
> - do tb_flush in breakpoint_invalidate unconditionally
I know I had reservations about this, but we now have two patches on list that
fix the problem in this way.
What I would *like* is for each CPUBreakpoint to maintain a list of the TBs to
which it has been applied, so that each can be invalidated. Our current
management of breakpoints are IMO sloppy.
That said, I don't really have time to work on cleaning this up myself in the
short term, and this is fixing a real bug. Therefore, I am going to queue this
to tcg-next.
I would still like patch 2/2 to be split, and that can probably go through an
xtensa branch.
r~
>
> exec.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index ffdb5185353b..1709b760edc1 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1017,14 +1017,13 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr
> addr, MemTxAttrs attrs)
>
> static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
> {
> - MemTxAttrs attrs;
> - hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, &attrs);
> - int asidx = cpu_asidx_from_attrs(cpu, attrs);
> - if (phys != -1) {
> - /* Locks grabbed by tb_invalidate_phys_addr */
> - tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as,
> - phys | (pc & ~TARGET_PAGE_MASK), attrs);
> - }
> + /*
> + * 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
>
>
