* [Qemu-devel] [PATCH] A bit optimization for tlb_set_page() @ 2010-04-27 3:25 Jun Koi 2010-04-27 18:36 ` [Qemu-devel] " Jan Kiszka 0 siblings, 1 reply; 7+ messages in thread From: Jun Koi @ 2010-04-27 3:25 UTC (permalink / raw) To: qemu-devel 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. Signed-off-by: Jun Koi <junkoi2004@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. */ address |= TLB_MMIO; + break; } } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] A bit optimization for tlb_set_page() 2010-04-27 3:25 [Qemu-devel] [PATCH] A bit optimization for tlb_set_page() Jun Koi @ 2010-04-27 18:36 ` Jan Kiszka 2010-04-27 23:48 ` Jun Koi 0 siblings, 1 reply; 7+ messages in thread From: Jan Kiszka @ 2010-04-27 18:36 UTC (permalink / raw) To: Jun Koi; +Cc: qemu-devel [-- Attachment #1: Type: text/plain, Size: 817 bytes --] 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.kiszka@siemens.com> > > Signed-off-by: Jun Koi <junkoi2004@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? :) > address |= TLB_MMIO; > + break; > } > } > > > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] A bit optimization for tlb_set_page() 2010-04-27 18:36 ` [Qemu-devel] " Jan Kiszka @ 2010-04-27 23:48 ` Jun Koi 2010-04-28 2:52 ` Jun Koi 0 siblings, 1 reply; 7+ messages in thread From: Jun Koi @ 2010-04-27 23:48 UTC (permalink / raw) To: Jan Kiszka; +Cc: qemu-devel On Wed, Apr 28, 2010 at 3:36 AM, Jan Kiszka <jan.kiszka@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.kiszka@siemens.com> > >> >> Signed-off-by: Jun Koi <junkoi2004@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? :) > OK, I will look at that in my little spare time. Thanks, J ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] A bit optimization for tlb_set_page() 2010-04-27 23:48 ` Jun Koi @ 2010-04-28 2:52 ` Jun Koi 2010-04-28 6:53 ` Jan Kiszka 0 siblings, 1 reply; 7+ messages in thread From: Jun Koi @ 2010-04-28 2:52 UTC (permalink / raw) To: Jan Kiszka; +Cc: qemu-devel On Wed, Apr 28, 2010 at 8:48 AM, Jun Koi <junkoi2004@gmail.com> wrote: > On Wed, Apr 28, 2010 at 3:36 AM, Jan Kiszka <jan.kiszka@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.kiszka@siemens.com> >> >>> >>> Signed-off-by: Jun Koi <junkoi2004@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 <junkoi2004@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))) { + iotlb = io_mem_watch + paddr; + address |= TLB_MMIO; + break; + } } } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] A bit optimization for tlb_set_page() 2010-04-28 2:52 ` Jun Koi @ 2010-04-28 6:53 ` Jan Kiszka 2010-04-28 7:13 ` Jun Koi 0 siblings, 1 reply; 7+ messages in thread From: Jan Kiszka @ 2010-04-28 6:53 UTC (permalink / raw) To: Jun Koi; +Cc: qemu-devel [-- Attachment #1: Type: text/plain, Size: 2273 bytes --] Jun Koi wrote: > On Wed, Apr 28, 2010 at 8:48 AM, Jun Koi <junkoi2004@gmail.com> wrote: >> On Wed, Apr 28, 2010 at 3:36 AM, Jan Kiszka <jan.kiszka@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.kiszka@siemens.com> >>> >>>> Signed-off-by: Jun Koi <junkoi2004@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 <junkoi2004@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. Besides that if ((wp->flags & BP_MEM_READ) || (prot & PAGE_WRITE)) is more readable IMHO. > + iotlb = io_mem_watch + paddr; > + address |= TLB_MMIO; > + break; > + } > } > } Thanks, Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] A bit optimization for tlb_set_page() 2010-04-28 6:53 ` Jan Kiszka @ 2010-04-28 7:13 ` Jun Koi 2010-04-30 6:44 ` Jan Kiszka 0 siblings, 1 reply; 7+ messages in thread From: Jun Koi @ 2010-04-28 7:13 UTC (permalink / raw) To: Jan Kiszka; +Cc: qemu-devel On Wed, Apr 28, 2010 at 3:53 PM, Jan Kiszka <jan.kiszka@web.de> wrote: > Jun Koi wrote: >> On Wed, Apr 28, 2010 at 8:48 AM, Jun Koi <junkoi2004@gmail.com> wrote: >>> On Wed, Apr 28, 2010 at 3:36 AM, Jan Kiszka <jan.kiszka@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.kiszka@siemens.com> >>>> >>>>> Signed-off-by: Jun Koi <junkoi2004@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 <junkoi2004@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 <junkoi2004@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; + } } } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] A bit optimization for tlb_set_page() 2010-04-28 7:13 ` Jun Koi @ 2010-04-30 6:44 ` Jan Kiszka 0 siblings, 0 replies; 7+ messages in thread From: Jan Kiszka @ 2010-04-30 6:44 UTC (permalink / raw) To: Jun Koi; +Cc: qemu-devel [-- Attachment #1: Type: text/plain, Size: 3682 bytes --] Jun Koi wrote: > On Wed, Apr 28, 2010 at 3:53 PM, Jan Kiszka <jan.kiszka@web.de> wrote: >> Jun Koi wrote: >>> On Wed, Apr 28, 2010 at 8:48 AM, Jun Koi <junkoi2004@gmail.com> wrote: >>>> On Wed, Apr 28, 2010 at 3:36 AM, Jan Kiszka <jan.kiszka@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.kiszka@siemens.com> >>>>> >>>>>> Signed-off-by: Jun Koi <junkoi2004@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 <junkoi2004@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. > Looks ok, but you will likely have to resend it with proper subject and without any traces of this thread. Otherwise the patch won't be found by any maintainer. Jan > 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 <junkoi2004@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; > + } > } > } [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-04-30 6:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-27 3:25 [Qemu-devel] [PATCH] A bit optimization for tlb_set_page() Jun Koi 2010-04-27 18:36 ` [Qemu-devel] " Jan Kiszka 2010-04-27 23:48 ` Jun Koi 2010-04-28 2:52 ` Jun Koi 2010-04-28 6:53 ` Jan Kiszka 2010-04-28 7:13 ` Jun Koi 2010-04-30 6:44 ` Jan Kiszka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).