From: David Hildenbrand <david@redhat.com> To: qemu-devel@nongnu.org Cc: Thomas Huth <thuth@redhat.com>, Riku Voipio <riku.voipio@iki.fi>, Eduardo Habkost <ehabkost@redhat.com>, Aleksandar Rikalo <arikalo@wavecomp.com>, Cornelia Huck <cohuck@redhat.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, qemu-s390x@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Richard Henderson <rth@twiddle.net> Subject: Re: [Qemu-devel] [PATCH v1 8/9] exec.c: Factor out core logic of check_watchpoint() Date: Fri, 23 Aug 2019 13:27:41 +0200 Message-ID: <8df7d599-bc35-621e-c5d9-ac8554cba512@redhat.com> (raw) In-Reply-To: <20190823100741.9621-9-david@redhat.com> On 23.08.19 12:07, David Hildenbrand wrote: > We want to perform the same checks in probe_write() to trigger a cpu > exit before doing any modifications. We'll have to pass a PC. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > exec.c | 23 +++++++++++++++++------ > include/hw/core/cpu.h | 2 ++ > 2 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/exec.c b/exec.c > index 1df966d17a..d233a4250b 100644 > --- a/exec.c > +++ b/exec.c > @@ -2810,12 +2810,10 @@ static const MemoryRegionOps notdirty_mem_ops = { > }, > }; > > -/* Generate a debug exception if a watchpoint has been hit. */ > -static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) > +void cpu_check_watchpoint(CPUState *cpu, vaddr vaddr, int len, > + MemTxAttrs attrs, int flags, uintptr_t ra) > { > - CPUState *cpu = current_cpu; > CPUClass *cc = CPU_GET_CLASS(cpu); > - target_ulong vaddr; > CPUWatchpoint *wp; > > assert(tcg_enabled()); > @@ -2826,7 +2824,7 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) > cpu_interrupt(cpu, CPU_INTERRUPT_DEBUG); > return; > } > - vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; > + > vaddr = cc->adjust_watchpoint_address(cpu, vaddr, len); > QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { > if (cpu_watchpoint_address_matches(wp, vaddr, len) > @@ -2851,11 +2849,14 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) > if (wp->flags & BP_STOP_BEFORE_ACCESS) { > cpu->exception_index = EXCP_DEBUG; > mmap_unlock(); > - cpu_loop_exit(cpu); > + cpu_loop_exit_restore(cpu, ra); > } else { > /* Force execution of one insn next time. */ > cpu->cflags_next_tb = 1 | curr_cflags(); > mmap_unlock(); > + if (ra) { > + cpu_restore_state(cpu, ra, true); > + } > cpu_loop_exit_noexc(cpu); > } > } > @@ -2865,6 +2866,16 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) > } > } > > +/* Generate a debug exception if a watchpoint has been hit. */ > +static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) > +{ > + CPUState *cpu = current_cpu; > + vaddr vaddr; > + > + vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; > + cpu_check_watchpoint(cpu, vaddr, len, attrs, flags, 0); > +} > + > /* Watchpoint access routines. Watchpoints are inserted using TLB tricks, > so these check for a hit then pass through to the normal out-of-line > phys routines. */ > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h > index 77fca95a40..3a2d76b32c 100644 > --- a/include/hw/core/cpu.h > +++ b/include/hw/core/cpu.h > @@ -1070,6 +1070,8 @@ static inline bool cpu_breakpoint_test(CPUState *cpu, vaddr pc, int mask) > return false; > } > > +void cpu_check_watchpoint(CPUState *cpu, vaddr vaddr, int len, > + MemTxAttrs attrs, int flags, uintptr_t ra); > int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, > int flags, CPUWatchpoint **watchpoint); > int cpu_watchpoint_remove(CPUState *cpu, vaddr addr, > As we will have bigger accesses with probe_write(), we should do diff --git a/exec.c b/exec.c index d233a4250b..4f8cc62a5f 100644 --- a/exec.c +++ b/exec.c @@ -2834,7 +2834,7 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr vaddr, int len, } else { wp->flags |= BP_WATCHPOINT_HIT_WRITE; } - wp->hitaddr = vaddr; + wp->hitaddr = MAX(vaddr, wp->vaddr); wp->hitattrs = attrs; if (!cpu->watchpoint_hit) { if (wp->flags & BP_CPU && I guess, to make sure we actually indicate the watchpoint. -- Thanks, David / dhildenb
next prev parent reply index Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-23 10:07 [Qemu-devel] [PATCH v1 0/9] tcg: probe_write() refactorings and watchpoints David Hildenbrand 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 1/9] s390x/tcg: Use guest_addr_valid() instead of h2g_valid() in probe_write_access() David Hildenbrand 2019-08-23 14:49 ` Richard Henderson 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 2/9] s390x/tcg: Fix length calculation " David Hildenbrand 2019-08-23 14:50 ` Richard Henderson 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 3/9] tcg: Factor out CONFIG_USER_ONLY probe_write() from s390x code David Hildenbrand 2019-08-23 15:17 ` Richard Henderson 2019-08-23 15:22 ` Richard Henderson 2019-08-23 15:49 ` David Hildenbrand 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 4/9] tcg: Enforce single page access in probe_write() for !CONFIG_USER_ONLY David Hildenbrand 2019-08-23 15:28 ` Richard Henderson 2019-08-23 15:29 ` Richard Henderson 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 5/9] mips/tcg: Call probe_write() for CONFIG_USER_ONLY as well David Hildenbrand 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 6/9] hppa/tcg: Call probe_write() also for CONFIG_USER_ONLY David Hildenbrand 2019-08-23 15:31 ` Richard Henderson 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 7/9] s390x/tcg: Pass a size to probe_write() in do_csst() David Hildenbrand 2019-08-23 15:32 ` Richard Henderson 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 8/9] exec.c: Factor out core logic of check_watchpoint() David Hildenbrand 2019-08-23 11:27 ` David Hildenbrand [this message] 2019-08-23 16:09 ` Richard Henderson 2019-08-23 18:25 ` David Hildenbrand 2019-08-24 15:27 ` Richard Henderson 2019-08-23 10:07 ` [Qemu-devel] [PATCH v1 9/9] tcg: Check for watchpoints in probe_write() David Hildenbrand 2019-08-23 16:15 ` Richard Henderson 2019-08-24 19:45 ` Richard Henderson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=8df7d599-bc35-621e-c5d9-ac8554cba512@redhat.com \ --to=david@redhat.com \ --cc=amarkovic@wavecomp.com \ --cc=arikalo@wavecomp.com \ --cc=aurelien@aurel32.net \ --cc=cohuck@redhat.com \ --cc=ehabkost@redhat.com \ --cc=pbonzini@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=qemu-s390x@nongnu.org \ --cc=riku.voipio@iki.fi \ --cc=rth@twiddle.net \ --cc=thuth@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
QEMU-Devel Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \ qemu-devel@nongnu.org public-inbox-index qemu-devel Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git