| Message ID | 20191126222607.25653-1-jcmvbkbc@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] exec: flush CPU TB cache when breakpoint address translation fails | expand |
On 26/11/19 23:26, 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 when a breakpoint is set for a virtual address > that cannot be translated to physical address. > > This change fixes the following workflow: > - 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. That's quite heavyweight, but perhaps since it's debugging we might as well _always_ just flush the TB cache? In any case, it deserves a comment in the code. Paolo
Max Filippov <jcmvbkbc@gmail.com> writes: > 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. So the userspace has run once but is currently paged out? > > Flush entire CPU TB cache when a breakpoint is set for a virtual address > that cannot be translated to physical address. > > This change fixes the following workflow: > - 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 <jcmvbkbc@gmail.com> > --- > exec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/exec.c b/exec.c > index ffdb5185353b..918945f8097e 100644 > --- a/exec.c > +++ b/exec.c > @@ -1024,6 +1024,8 @@ static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) > /* Locks grabbed by tb_invalidate_phys_addr */ > tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as, > phys | (pc & ~TARGET_PAGE_MASK), attrs); > + } else { > + tb_flush(cpu); > } > } > #endif
On Wed, Nov 27, 2019 at 11:06 AM Alex Bennée <alex.bennee@linaro.org> wrote: > Max Filippov <jcmvbkbc@gmail.com> writes: > > > 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. > > So the userspace has run once but is currently paged out? Yes, but not necessarily paged out, just not in the CPU TLB. Or it has run to completion and when you start it next time it gets loaded to the same physical pages.
On 11/26/19 10:26 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 when a breakpoint is set for a virtual address > that cannot be translated to physical address. > > This change fixes the following workflow: > - 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 <jcmvbkbc@gmail.com> As a short-term fix this works. I'm willing to apply this to 5.0 with cc: stable so that as a bug fix this gets pulled back to 4.2.1. Paolo's request for a comment is en pointe, so I'll wait for a version 2. However, while running in system mode with a decent sized amount of guest ram, I see 1-2M tbs live at once. We don't really want to throw all of those away. I think it would be better to put the tbs into some sort of data structure that indexes the tbs by their virtual address. I have no strong feelings about what sort of data structure would be best, just something better than a linear search of all of the tbs. :-) r~
diff --git a/exec.c b/exec.c index ffdb5185353b..918945f8097e 100644 --- a/exec.c +++ b/exec.c @@ -1024,6 +1024,8 @@ static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) /* Locks grabbed by tb_invalidate_phys_addr */ tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as, phys | (pc & ~TARGET_PAGE_MASK), attrs); + } else { + tb_flush(cpu); } } #endif
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 when a breakpoint is set for a virtual address that cannot be translated to physical address. This change fixes the following workflow: - 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 <jcmvbkbc@gmail.com> --- exec.c | 2 ++ 1 file changed, 2 insertions(+)