On Wed, Apr 28, 2010 at 3:53 PM, Jan Kiszka <jan.kis...@web.de> wrote: > Jun Koi wrote: >> On Wed, Apr 28, 2010 at 8:48 AM, Jun Koi <junkoi2...@gmail.com> wrote: >>> On Wed, Apr 28, 2010 at 3:36 AM, Jan Kiszka <jan.kis...@web.de> wrote: >>>> Jun Koi wrote: >>>>> It is not necessary to continue searching for watchpoint when we >>>>> already found one and setup for handling watchpoint in a search loop >>>>> in tlb_set_page(). >>>>> This patch breaks that search loop on then. >>>> Acked-by: Jan Kiszka <jan.kis...@siemens.com> >>>> >>>>> Signed-off-by: Jun Koi <junkoi2...@gmail.com> >>>>> >>>>> diff --git a/exec.c b/exec.c >>>>> index 14d1fd7..6329775 100644 >>>>> --- a/exec.c >>>>> +++ b/exec.c >>>>> @@ -2240,6 +2240,7 @@ void tlb_set_page(CPUState *env, target_ulong vaddr, >>>>> /* TODO: The memory case can be optimized by not trapping >>>>> reads of pages with a write breakpoint. */ >>>> PS: Don't you also want to address this todo while you are at it? :) >>>> >> >> Do you have any comment on below patch? >> >> Thanks, >> J >> >> >> Dont handle write watchpoints on read-only memory access. >> This patch also breaks the searching loop for watchpoint once the >> setup facilities for handling watchpoint later is done. >> >> Signed-off-by: Jun Koi <junkoi2...@gmail.com> >> >> diff --git a/exec.c b/exec.c >> index 14d1fd7..3f2d9ed 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -2236,10 +2236,12 @@ void tlb_set_page(CPUState *env, target_ulong vaddr, >> watchpoint trap routines. */ >> QTAILQ_FOREACH(wp, &env->watchpoints, entry) { >> if (vaddr == (wp->vaddr & TARGET_PAGE_MASK)) { >> - iotlb = io_mem_watch + paddr; >> - /* TODO: The memory case can be optimized by not trapping >> - reads of pages with a write breakpoint. */ >> - address |= TLB_MMIO; >> + /* Avoid trapping reads of pages with a write breakpoint. */ >> + if (!((prot & PAGE_WRITE == 0) && (wp->flags & >> BP_MEM_WRITE != 0))) { > > Patch is line-wrapped.
That is the fault of Gmail. > Besides that > > if ((wp->flags & BP_MEM_READ) || (prot & PAGE_WRITE)) > > is more readable IMHO. Agreed. Please see my updated patch. Thanks, Jun Dont handle write watchpoints on read-only memory access. This patch also breaks the searching loop for watchpoint once the setup for handling watchpoint later is done. Signed-off-by: Jun Koi <junkoi2...@gmail.com> diff --git a/exec.c b/exec.c index 14d1fd7..6fd859f 100644 --- a/exec.c +++ b/exec.c @@ -2236,10 +2236,12 @@ void tlb_set_page(CPUState *env, target_ulong vaddr, watchpoint trap routines. */ QTAILQ_FOREACH(wp, &env->watchpoints, entry) { if (vaddr == (wp->vaddr & TARGET_PAGE_MASK)) { - iotlb = io_mem_watch + paddr; - /* TODO: The memory case can be optimized by not trapping - reads of pages with a write breakpoint. */ - address |= TLB_MMIO; + /* Avoid trapping reads of pages with a write breakpoint. */ + if ((prot & PAGE_WRITE) || (wp->flags & BP_MEM_READ)) { + iotlb = io_mem_watch + paddr; + address |= TLB_MMIO; + break; + } } }