| Message ID | CAETRQW=Yxwh5LCG4=To-dDUUVf-SktQuP3ctUt2_N7P-fM0+Hg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Friday, May 18, 2012, TeLeMan <geleman@gmail.com> wrote: > This breakage was introduced by the commit "memory: make > phys_page_find() return an unadjusted". > > Signed-off-by: TeLeMan <geleman@gmail.com> > --- > exec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 0607c9b..ad99476 100644 > --- a/exec.c > +++ b/exec.c > @@ -1475,7 +1475,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr) > > static void breakpoint_invalidate(CPUArchState *env, target_ulong pc) > { > - tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc)); > + tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc) > + | (pc & ~TARGET_PAGE_MASK)); > } > #endif > #endif /* TARGET_HAS_ICE */ > -- > 1.7.10.msysgit.1 > ping?
Am 18.05.2012 11:49, schrieb TeLeMan: > This breakage was introduced by the commit "memory: make > phys_page_find() return an unadjusted". You seem to have found the origin of your problem. If you also mention the commit hash in your commit message then certain frontends (gitk, repo.or.cz) will display it as a handy hyperlink to that commit. > > Signed-off-by: TeLeMan <geleman@gmail.com> Signed-off-by is a legal statement of origin and must not be a pseudonym. /-F > --- > exec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 0607c9b..ad99476 100644 > --- a/exec.c > +++ b/exec.c > @@ -1475,7 +1475,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr) > > static void breakpoint_invalidate(CPUArchState *env, target_ulong pc) > { > - tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc)); > + tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc) > + | (pc & ~TARGET_PAGE_MASK)); > } > #endif > #endif /* TARGET_HAS_ICE */
On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: > Am 18.05.2012 11:49, schrieb TeLeMan: >> This breakage was introduced by the commit "memory: make >> phys_page_find() return an unadjusted". > > You seem to have found the origin of your problem. If you also mention > the commit hash in your commit message then certain frontends (gitk, > repo.or.cz) will display it as a handy hyperlink to that commit. > >> >> Signed-off-by: TeLeMan <geleman@gmail.com> > > Signed-off-by is a legal statement of origin and must not be a pseudonym. Ok, please ignore this patch. I won't submit any patch just report bugs. > /-F > >> --- >> exec.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/exec.c b/exec.c >> index 0607c9b..ad99476 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1475,7 +1475,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr) >> >> static void breakpoint_invalidate(CPUArchState *env, target_ulong pc) >> { >> - tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc)); >> + tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc) >> + | (pc & ~TARGET_PAGE_MASK)); >> } >> #endif >> #endif /* TARGET_HAS_ICE */ > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
On 2012-05-23 04:09, TeLeMan wrote: > On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: >> Am 18.05.2012 11:49, schrieb TeLeMan: >>> This breakage was introduced by the commit "memory: make >>> phys_page_find() return an unadjusted". >> >> You seem to have found the origin of your problem. If you also mention >> the commit hash in your commit message then certain frontends (gitk, >> repo.or.cz) will display it as a handy hyperlink to that commit. >> >>> >>> Signed-off-by: TeLeMan <geleman@gmail.com> >> >> Signed-off-by is a legal statement of origin and must not be a pseudonym. > Ok, please ignore this patch. I won't submit any patch just report bugs. Then please describe this bug in more details, e.g. how to reproduce. Thanks, Jan
On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2012-05-23 04:09, TeLeMan wrote: >> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: >>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>> This breakage was introduced by the commit "memory: make >>>> phys_page_find() return an unadjusted". >>> >>> You seem to have found the origin of your problem. If you also mention >>> the commit hash in your commit message then certain frontends (gitk, >>> repo.or.cz) will display it as a handy hyperlink to that commit. >>> >>>> >>>> Signed-off-by: TeLeMan <geleman@gmail.com> >>> >>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >> Ok, please ignore this patch. I won't submit any patch just report bugs. > > Then please describe this bug in more details, e.g. how to reproduce. I think its evident. cpu_get_phys_page_debug(env, pc) is not the physical address of pc but the physical page base address of pc. > Thanks, > Jan > > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux
On 2012-05-23 11:11, TeLeMan wrote: > On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2012-05-23 04:09, TeLeMan wrote: >>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: >>>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>>> This breakage was introduced by the commit "memory: make >>>>> phys_page_find() return an unadjusted". >>>> >>>> You seem to have found the origin of your problem. If you also mention >>>> the commit hash in your commit message then certain frontends (gitk, >>>> repo.or.cz) will display it as a handy hyperlink to that commit. >>>> >>>>> >>>>> Signed-off-by: TeLeMan <geleman@gmail.com> >>>> >>>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >>> Ok, please ignore this patch. I won't submit any patch just report bugs. >> >> Then please describe this bug in more details, e.g. how to reproduce. > I think its evident. cpu_get_phys_page_debug(env, pc) is not the > physical address of pc but the physical page base address of pc. ...so this bites us if the instruction spans two pages as tb_invalidate_phys_addr requests invalidation on a page granularity. Such information would have been helpful to understand when it actually breaks - not in the common case. Thanks, Jan
On 2012-05-23 13:02, Jan Kiszka wrote: > On 2012-05-23 11:11, TeLeMan wrote: >> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> On 2012-05-23 04:09, TeLeMan wrote: >>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: >>>>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>>>> This breakage was introduced by the commit "memory: make >>>>>> phys_page_find() return an unadjusted". >>>>> >>>>> You seem to have found the origin of your problem. If you also mention >>>>> the commit hash in your commit message then certain frontends (gitk, >>>>> repo.or.cz) will display it as a handy hyperlink to that commit. >>>>> >>>>>> >>>>>> Signed-off-by: TeLeMan <geleman@gmail.com> >>>>> >>>>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >>>> Ok, please ignore this patch. I won't submit any patch just report bugs. >>> >>> Then please describe this bug in more details, e.g. how to reproduce. >> I think its evident. cpu_get_phys_page_debug(env, pc) is not the >> physical address of pc but the physical page base address of pc. > > ...so this bites us if the instruction spans two pages as > tb_invalidate_phys_addr requests invalidation on a page granularity. In fact, this is irrelevant. We only need to flush the address at which the instruction starts, and that is achieved by flushing all TB that relate to that page as the current code does. So, again my question: How can I reproduce the issue you see? Jan
On Wed, May 23, 2012 at 3:41 AM, Andreas Färber <afaerber@suse.de> wrote: > Am 18.05.2012 11:49, schrieb TeLeMan: >> This breakage was introduced by the commit "memory: make >> phys_page_find() return an unadjusted". > > You seem to have found the origin of your problem. If you also mention > the commit hash in your commit message then certain frontends (gitk, > repo.or.cz) will display it as a handy hyperlink to that commit. > >> >> Signed-off-by: TeLeMan <geleman@gmail.com> > > Signed-off-by is a legal statement of origin and must not be a pseudonym. $ git log --author=.*eleman commit c62f6d1d76aea587556c85b6b7b5c44167006264 Author: TeLeMan <geleman@gmail.com> Date: Mon Jul 25 16:29:14 2011 +0800 monitor: fix build breakage with --disable-vnc The breakage was introduced by the commit 13661089810d3e59931f3e80d7cb541b99af7071 Signed-off-by: TeLeMan <geleman@gmail.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> and several others. > > /-F > >> --- >> exec.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/exec.c b/exec.c >> index 0607c9b..ad99476 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1475,7 +1475,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr) >> >> static void breakpoint_invalidate(CPUArchState *env, target_ulong pc) >> { >> - tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc)); >> + tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc) >> + | (pc & ~TARGET_PAGE_MASK)); >> } >> #endif >> #endif /* TARGET_HAS_ICE */ > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
Am 23.05.2012 21:40, schrieb Blue Swirl: > On Wed, May 23, 2012 at 3:41 AM, Andreas Färber <afaerber@suse.de> wrote: >> Am 18.05.2012 11:49, schrieb TeLeMan: >>> This breakage was introduced by the commit "memory: make >>> phys_page_find() return an unadjusted". >> >> You seem to have found the origin of your problem. If you also mention >> the commit hash in your commit message then certain frontends (gitk, >> repo.or.cz) will display it as a handy hyperlink to that commit. >> >>> >>> Signed-off-by: TeLeMan <geleman@gmail.com> >> >> Signed-off-by is a legal statement of origin and must not be a pseudonym. > > $ git log --author=.*eleman > commit c62f6d1d76aea587556c85b6b7b5c44167006264 > Author: TeLeMan <geleman@gmail.com> > Date: Mon Jul 25 16:29:14 2011 +0800 > > monitor: fix build breakage with --disable-vnc > > The breakage was introduced by the commit > 13661089810d3e59931f3e80d7cb541b99af7071 > > Signed-off-by: TeLeMan <geleman@gmail.com> > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > > and several others. Funny, since I learned that from Anthony. :-) Anyway, the only non-childish reason to contribute under a pseudonym (as opposed to a known alias*) I can imagine is a contribution from someone working on a competing commercial emulation/virtualization solution who doesn't want her employer finding out, in which case it is questionable whether she can actually sign off her own code as it may conflict with non-free company IP. Any comments, Mr. Blauwirbel? ;) Andreas * e.g., malc, kraxel, mmu_man
On Wed, May 23, 2012 at 8:04 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 23.05.2012 21:40, schrieb Blue Swirl: >> On Wed, May 23, 2012 at 3:41 AM, Andreas Färber <afaerber@suse.de> wrote: >>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>> This breakage was introduced by the commit "memory: make >>>> phys_page_find() return an unadjusted". >>> >>> You seem to have found the origin of your problem. If you also mention >>> the commit hash in your commit message then certain frontends (gitk, >>> repo.or.cz) will display it as a handy hyperlink to that commit. >>> >>>> >>>> Signed-off-by: TeLeMan <geleman@gmail.com> >>> >>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >> >> $ git log --author=.*eleman >> commit c62f6d1d76aea587556c85b6b7b5c44167006264 >> Author: TeLeMan <geleman@gmail.com> >> Date: Mon Jul 25 16:29:14 2011 +0800 >> >> monitor: fix build breakage with --disable-vnc >> >> The breakage was introduced by the commit >> 13661089810d3e59931f3e80d7cb541b99af7071 >> >> Signed-off-by: TeLeMan <geleman@gmail.com> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> >> and several others. > > Funny, since I learned that from Anthony. :-) > > Anyway, the only non-childish reason to contribute under a pseudonym (as > opposed to a known alias*) I can imagine is a contribution from someone > working on a competing commercial emulation/virtualization solution who > doesn't want her employer finding out, in which case it is questionable > whether she can actually sign off her own code as it may conflict with > non-free company IP. Any comments, Mr. Blauwirbel? ;) I don't think the reason you imagined is very probable and I can imagine other reasons. ;-) > > Andreas > > * e.g., malc, kraxel, mmu_man > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
On 05/23/2012 03:28 PM, Blue Swirl wrote: > On Wed, May 23, 2012 at 8:04 PM, Andreas Färber<afaerber@suse.de> wrote: >> Am 23.05.2012 21:40, schrieb Blue Swirl: >>> On Wed, May 23, 2012 at 3:41 AM, Andreas Färber<afaerber@suse.de> wrote: >>>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>>> This breakage was introduced by the commit "memory: make >>>>> phys_page_find() return an unadjusted". >>>> >>>> You seem to have found the origin of your problem. If you also mention >>>> the commit hash in your commit message then certain frontends (gitk, >>>> repo.or.cz) will display it as a handy hyperlink to that commit. >>>> >>>>> >>>>> Signed-off-by: TeLeMan<geleman@gmail.com> >>>> >>>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >>> >>> $ git log --author=.*eleman >>> commit c62f6d1d76aea587556c85b6b7b5c44167006264 >>> Author: TeLeMan<geleman@gmail.com> >>> Date: Mon Jul 25 16:29:14 2011 +0800 >>> >>> monitor: fix build breakage with --disable-vnc >>> >>> The breakage was introduced by the commit >>> 13661089810d3e59931f3e80d7cb541b99af7071 >>> >>> Signed-off-by: TeLeMan<geleman@gmail.com> >>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >>> >>> and several others. >> >> Funny, since I learned that from Anthony. :-) Signed-off-by is a contractual statement and your real name is required: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches#l339 But I'm generally going to assume that if you SoB something, it's your real name. I have challenged people in the past but only when it's very obviously fake. If TeLeMan is not your real name, you cannot Signed-off-by with it. Regards, Anthony Liguori
On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2012-05-23 13:02, Jan Kiszka wrote: >> On 2012-05-23 11:11, TeLeMan wrote: >>> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>> On 2012-05-23 04:09, TeLeMan wrote: >>>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: >>>>>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>>>>> This breakage was introduced by the commit "memory: make >>>>>>> phys_page_find() return an unadjusted". >>>>>> >>>>>> You seem to have found the origin of your problem. If you also mention >>>>>> the commit hash in your commit message then certain frontends (gitk, >>>>>> repo.or.cz) will display it as a handy hyperlink to that commit. >>>>>> >>>>>>> >>>>>>> Signed-off-by: TeLeMan <geleman@gmail.com> >>>>>> >>>>>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >>>>> Ok, please ignore this patch. I won't submit any patch just report bugs. >>>> >>>> Then please describe this bug in more details, e.g. how to reproduce. >>> I think its evident. cpu_get_phys_page_debug(env, pc) is not the >>> physical address of pc but the physical page base address of pc. >> >> ...so this bites us if the instruction spans two pages as >> tb_invalidate_phys_addr requests invalidation on a page granularity. > > In fact, this is irrelevant. We only need to flush the address at which > the instruction starts, and that is achieved by flushing all TB that > relate to that page as the current code does. But the instruction start is wrong and its TB may not be found. For example, the pc is 0x1234 and its physical address is 0x1234. The correct "start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and 0x1235. But now the "start" and "end" is 0x1000 and 0x1001. If 0x1000 is not translated yet, the real TB won't be invalidated. > So, again my question: How can I reproduce the issue you see? > > Jan > > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux
On 2012-05-23 22:29, TeLeMan wrote: > On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2012-05-23 13:02, Jan Kiszka wrote: >>> On 2012-05-23 11:11, TeLeMan wrote: >>>> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>> On 2012-05-23 04:09, TeLeMan wrote: >>>>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: >>>>>>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>>>>>> This breakage was introduced by the commit "memory: make >>>>>>>> phys_page_find() return an unadjusted". >>>>>>> >>>>>>> You seem to have found the origin of your problem. If you also mention >>>>>>> the commit hash in your commit message then certain frontends (gitk, >>>>>>> repo.or.cz) will display it as a handy hyperlink to that commit. >>>>>>> >>>>>>>> >>>>>>>> Signed-off-by: TeLeMan <geleman@gmail.com> >>>>>>> >>>>>>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >>>>>> Ok, please ignore this patch. I won't submit any patch just report bugs. >>>>> >>>>> Then please describe this bug in more details, e.g. how to reproduce. >>>> I think its evident. cpu_get_phys_page_debug(env, pc) is not the >>>> physical address of pc but the physical page base address of pc. >>> >>> ...so this bites us if the instruction spans two pages as >>> tb_invalidate_phys_addr requests invalidation on a page granularity. >> >> In fact, this is irrelevant. We only need to flush the address at which >> the instruction starts, and that is achieved by flushing all TB that >> relate to that page as the current code does. > > But the instruction start is wrong and its TB may not be found. For example, > the pc is 0x1234 and its physical address is 0x1234. The correct > "start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and > 0x1235. But now the "start" and "end" is 0x1000 and 0x1001. > If 0x1000 is not translated yet, the real TB won't be invalidated. The tb containing 0x1234 would be linked to the list of TBs that are related to the 0x1000 page. As we declare that page invalid, all affected TBs are dropped, not just the one containing the breakpoint. See tb_invalidate_phys_page_range. Jan
On Thu, May 24, 2012 at 4:44 AM, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 05/23/2012 03:28 PM, Blue Swirl wrote: >> >> On Wed, May 23, 2012 at 8:04 PM, Andreas Färber<afaerber@suse.de> wrote: >>> >>> Am 23.05.2012 21:40, schrieb Blue Swirl: >>>> >>>> On Wed, May 23, 2012 at 3:41 AM, Andreas Färber<afaerber@suse.de> >>>> wrote: >>>>> >>>>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>>>> >>>>>> This breakage was introduced by the commit "memory: make >>>>>> phys_page_find() return an unadjusted". >>>>> >>>>> >>>>> You seem to have found the origin of your problem. If you also mention >>>>> the commit hash in your commit message then certain frontends (gitk, >>>>> repo.or.cz) will display it as a handy hyperlink to that commit. >>>>> >>>>>> >>>>>> Signed-off-by: TeLeMan<geleman@gmail.com> >>>>> >>>>> >>>>> Signed-off-by is a legal statement of origin and must not be a >>>>> pseudonym. >>>> >>>> >>>> $ git log --author=.*eleman >>>> commit c62f6d1d76aea587556c85b6b7b5c44167006264 >>>> Author: TeLeMan<geleman@gmail.com> >>>> Date: Mon Jul 25 16:29:14 2011 +0800 >>>> >>>> monitor: fix build breakage with --disable-vnc >>>> >>>> The breakage was introduced by the commit >>>> 13661089810d3e59931f3e80d7cb541b99af7071 >>>> >>>> Signed-off-by: TeLeMan<geleman@gmail.com> >>>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >>>> >>>> and several others. >>> >>> >>> Funny, since I learned that from Anthony. :-) > > > Signed-off-by is a contractual statement and your real name is required: > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches#l339 > > But I'm generally going to assume that if you SoB something, it's your real > name. I have challenged people in the past but only when it's very > obviously fake. If TeLeMan is not your real name, you cannot Signed-off-by > with it. Sorry, I did't notice this rule before. Now I don't want to violate it. But I confuse myself if TeLeMan can be as my real name. Actually TeLeMan is not my Chinese name. I used TeLeMan as my nickname and English name. In other words TeLeMan can identify myself. I won't use my Chinese name because I think it's my privacy. I used QEMU to do my researching tools not as Andreas imagined. > Regards, > > Anthony Liguori
On Thu, May 24, 2012 at 10:00 AM, Jan Kiszka <jan.kiszka@web.de> wrote: > On 2012-05-23 22:29, TeLeMan wrote: >> On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> On 2012-05-23 13:02, Jan Kiszka wrote: >>>> On 2012-05-23 11:11, TeLeMan wrote: >>>>> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>> On 2012-05-23 04:09, TeLeMan wrote: >>>>>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: >>>>>>>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>>>>>>> This breakage was introduced by the commit "memory: make >>>>>>>>> phys_page_find() return an unadjusted". >>>>>>>> >>>>>>>> You seem to have found the origin of your problem. If you also mention >>>>>>>> the commit hash in your commit message then certain frontends (gitk, >>>>>>>> repo.or.cz) will display it as a handy hyperlink to that commit. >>>>>>>> >>>>>>>>> >>>>>>>>> Signed-off-by: TeLeMan <geleman@gmail.com> >>>>>>>> >>>>>>>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >>>>>>> Ok, please ignore this patch. I won't submit any patch just report bugs. >>>>>> >>>>>> Then please describe this bug in more details, e.g. how to reproduce. >>>>> I think its evident. cpu_get_phys_page_debug(env, pc) is not the >>>>> physical address of pc but the physical page base address of pc. >>>> >>>> ...so this bites us if the instruction spans two pages as >>>> tb_invalidate_phys_addr requests invalidation on a page granularity. >>> >>> In fact, this is irrelevant. We only need to flush the address at which >>> the instruction starts, and that is achieved by flushing all TB that >>> relate to that page as the current code does. >> >> But the instruction start is wrong and its TB may not be found. For example, >> the pc is 0x1234 and its physical address is 0x1234. The correct >> "start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and >> 0x1235. But now the "start" and "end" is 0x1000 and 0x1001. >> If 0x1000 is not translated yet, the real TB won't be invalidated. > > The tb containing 0x1234 would be linked to the list of TBs that are > related to the 0x1000 page. As we declare that page invalid, all > affected TBs are dropped, not just the one containing the breakpoint. > See tb_invalidate_phys_page_range. if (!(tb_end <= start || tb_start >= end)) { ... tb_phys_invalidate(tb, -1); ... } If start is wrong, tb_phys_invalidate() may not be called. > Jan >
On 2012-05-23 23:00, Jan Kiszka wrote: > On 2012-05-23 22:29, TeLeMan wrote: >> On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> On 2012-05-23 13:02, Jan Kiszka wrote: >>>> On 2012-05-23 11:11, TeLeMan wrote: >>>>> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>>>>> On 2012-05-23 04:09, TeLeMan wrote: >>>>>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber <afaerber@suse.de> wrote: >>>>>>>> Am 18.05.2012 11:49, schrieb TeLeMan: >>>>>>>>> This breakage was introduced by the commit "memory: make >>>>>>>>> phys_page_find() return an unadjusted". >>>>>>>> >>>>>>>> You seem to have found the origin of your problem. If you also mention >>>>>>>> the commit hash in your commit message then certain frontends (gitk, >>>>>>>> repo.or.cz) will display it as a handy hyperlink to that commit. >>>>>>>> >>>>>>>>> >>>>>>>>> Signed-off-by: TeLeMan <geleman@gmail.com> >>>>>>>> >>>>>>>> Signed-off-by is a legal statement of origin and must not be a pseudonym. >>>>>>> Ok, please ignore this patch. I won't submit any patch just report bugs. >>>>>> >>>>>> Then please describe this bug in more details, e.g. how to reproduce. >>>>> I think its evident. cpu_get_phys_page_debug(env, pc) is not the >>>>> physical address of pc but the physical page base address of pc. >>>> >>>> ...so this bites us if the instruction spans two pages as >>>> tb_invalidate_phys_addr requests invalidation on a page granularity. >>> >>> In fact, this is irrelevant. We only need to flush the address at which >>> the instruction starts, and that is achieved by flushing all TB that >>> relate to that page as the current code does. >> >> But the instruction start is wrong and its TB may not be found. For example, >> the pc is 0x1234 and its physical address is 0x1234. The correct >> "start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and >> 0x1235. But now the "start" and "end" is 0x1000 and 0x1001. >> If 0x1000 is not translated yet, the real TB won't be invalidated. > > The tb containing 0x1234 would be linked to the list of TBs that are > related to the 0x1000 page. As we declare that page invalid, all > affected TBs are dropped, not just the one containing the breakpoint. > See tb_invalidate_phys_page_range. Oops, too fast: in fact the introductory comment of tb_invalidate_phys_page_range is misleading, there is a sub-page-level range check. And now my test also actually triggers. Was probably running the wrong qemu version before. Jan
Am 24.05.2012 04:12, schrieb TeLeMan: > I won't use > my Chinese name because I think it's my privacy. That exactly is touching the core point: Signed-off-by is about transparency and taking responsibility for your actions, not hiding in anonymity. It's a certification of whom the code came from and who would be to blame if anything was wrong with that (think non-GPL-compatible code taken from somewhere else). Why should you be granted more privacy than us just because your name is Chinese? There's quite a few Chinese IBM guys around that don't seem to have any problem with this, and git would even handle UTF-8 characters quite well if desired[*]. Andreas [*] For example, commit e965fc380703110e967febf8d5b2ecd7db53b5d2 Author: 陳韋任 <chenwj@iis.sinica.edu.tw> Date: Mon Feb 6 14:02:55 2012 +0800 cpu-exec.c: Correct comment about this file and indentation cleanup Each target uses the #define macro (in target-xxx/cpu.h) to rename cpu_exec (cpu-exec.c) to cpu_xxx_exec, then defines its own cpu_loop which calls cpu_xxx_exec. So basically, cpu-exec.c is not only the i386 emulator main execution loop. This patch corrects the comment of this file and does indentation cleanup. Signed-off-by: Chen Wei-Ren (陳韋任) <chenwj@iis.sinica.edu.tw> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Am 24.05.2012 15:35, schrieb Andreas Färber: > Am 24.05.2012 04:12, schrieb TeLeMan: >> I won't use >> my Chinese name because I think it's my privacy. > > That exactly is touching the core point: Signed-off-by is about > transparency and taking responsibility for your actions, not hiding in > anonymity. It's a certification of whom the code came from and who would > be to blame if anything was wrong with that (think non-GPL-compatible > code taken from somewhere else). > > Why should you be granted more privacy than us just because your name is > Chinese? There's quite a few Chinese IBM guys around that don't seem to > have any problem with this, and git would even handle UTF-8 characters > quite well if desired[*]. > > Andreas > > > [*] For example, > > commit e965fc380703110e967febf8d5b2ecd7db53b5d2 > Author: 陳韋任<chenwj@iis.sinica.edu.tw> > Date: Mon Feb 6 14:02:55 2012 +0800 > > cpu-exec.c: Correct comment about this file and indentation cleanup > > Each target uses the #define macro (in target-xxx/cpu.h) to rename > cpu_exec (cpu-exec.c) to cpu_xxx_exec, then defines its own cpu_loop > which calls cpu_xxx_exec. So basically, cpu-exec.c is not only the i386 > emulator main execution loop. This patch corrects the comment of this > file and does indentation cleanup. > > Signed-off-by: Chen Wei-Ren (陳韋任)<chenwj@iis.sinica.edu.tw> > Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com> This discussion looks strange for me. If someone contributes to QEMU not only with patches but also on qemu-devel and in other forms since a long time, I appreciate his/her contributions, no matter what his/her name is. Names are not absolute. There are cultural differences with regard to names and their meaning - just think of the way names change when someone gets married. There are a lot of good and acceptable reasons why someone chooses to use a nickname or a pseudonym. If we strictly enforce real names, some people would simply choose a pseudonym. Maybe others would stop contributing. Therefore I suggest a pragmatical approach: if someone contributes for the first time using a nickname, it is good practice to tell that person that a real name is wanted. I would not insist on a real name, because there is no way to verify it, but verify such patches more carefully. When people contribute using a nick name for a long time, that nick _is_ their name, and I know them by that name as I know Andreas, Anthony or Avi. Blue and TeLeMan are well known names. I did not review the patch, but if the code is ok, I suggest to apply it. Regards, Stefan W.
On 05/24/2012 01:12 PM, Stefan Weil wrote: > Am 24.05.2012 15:35, schrieb Andreas Färber: >> Am 24.05.2012 04:12, schrieb TeLeMan: >>> I won't use >>> my Chinese name because I think it's my privacy. >> >> That exactly is touching the core point: Signed-off-by is about >> transparency and taking responsibility for your actions, not hiding in >> anonymity. It's a certification of whom the code came from and who would >> be to blame if anything was wrong with that (think non-GPL-compatible >> code taken from somewhere else). >> >> Why should you be granted more privacy than us just because your name is >> Chinese? There's quite a few Chinese IBM guys around that don't seem to >> have any problem with this, and git would even handle UTF-8 characters >> quite well if desired[*]. >> >> Andreas >> >> >> [*] For example, >> >> commit e965fc380703110e967febf8d5b2ecd7db53b5d2 >> Author: 陳韋任<chenwj@iis.sinica.edu.tw> >> Date: Mon Feb 6 14:02:55 2012 +0800 >> >> cpu-exec.c: Correct comment about this file and indentation cleanup >> >> Each target uses the #define macro (in target-xxx/cpu.h) to rename >> cpu_exec (cpu-exec.c) to cpu_xxx_exec, then defines its own cpu_loop >> which calls cpu_xxx_exec. So basically, cpu-exec.c is not only the i386 >> emulator main execution loop. This patch corrects the comment of this >> file and does indentation cleanup. >> >> Signed-off-by: Chen Wei-Ren (陳韋任)<chenwj@iis.sinica.edu.tw> >> Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com> > > This discussion looks strange for me. I'm not going to commit patches with a Signed-off-by if I know the name is an alias. DCO requires the use of a real name. DCO is an important part of ensuring the pedigree of a code base just like copyright licensing. Regards, Anthony Liguori
Am 24.05.2012 20:36, schrieb Anthony Liguori: > On 05/24/2012 01:12 PM, Stefan Weil wrote: >> This discussion looks strange for me. > > I'm not going to commit patches with a Signed-off-by if I know the > name is an alias. > > DCO requires the use of a real name. DCO is an important part of > ensuring the pedigree of a code base just like copyright licensing. > > Regards, > > Anthony Liguori > As I wrote in my previous mail, (personal) names are not absolute, but influenced by cultural facts. DCO was something written by lawyers from the western culture where individual personal names exist since some time and where these names are usually more or less constant during the life of a person. AFAIK, DCO was introduced for legal reasons (SCO => DCO). It would be interesting to know how many names used in Linux commits since DCO are faked real names. Without enforcing certified signatures, enforcing "real names" (that's names which look like some name you know) does not really ensure something. Nor does the requirement of a valid mail address. It is only an alibi pedigree. Even in America and Europe, the concept of a "real name" is rather new (only some hundred years old), and there still exist cultures without personal names. See http://en.wikipedia.org/wiki/Personal_name for more information. People change their name even today when they migrate from one country to another (many US immigrants did this, too) or when they change their religion (remember Cassius Clay?). Nevertheless I can understand that you are bound by the requirements of your employer. Regards, Steve (that's the name by which I was called when I was a youngster)
On 05/24/2012 02:42 PM, Stefan Weil wrote: > Am 24.05.2012 20:36, schrieb Anthony Liguori: >> On 05/24/2012 01:12 PM, Stefan Weil wrote: >>> This discussion looks strange for me. >> >> I'm not going to commit patches with a Signed-off-by if I know the name is an >> alias. >> >> DCO requires the use of a real name. DCO is an important part of ensuring the >> pedigree of a code base just like copyright licensing. >> >> Regards, >> >> Anthony Liguori >> > > As I wrote in my previous mail, (personal) names are not absolute, > but influenced by cultural facts. I understand this. And this is why I generally don't challenge the names people use with Signed-off-by. But when someone says that what they're using is not their legal name and they will not share their legal name, I have no choice but to not take patches from that person. Regards, Anthony Liguori > DCO was something written by > lawyers from the western culture where individual personal names > exist since some time and where these names are usually > more or less constant during the life of a person. AFAIK, > DCO was introduced for legal reasons (SCO => DCO). > > It would be interesting to know how many names used in Linux > commits since DCO are faked real names. Without enforcing > certified signatures, enforcing "real names" (that's names which > look like some name you know) does not really ensure something. > Nor does the requirement of a valid mail address. > It is only an alibi pedigree. > > Even in America and Europe, the concept of a "real name" is > rather new (only some hundred years old), and there still exist cultures > without personal names. See http://en.wikipedia.org/wiki/Personal_name > for more information. People change their name even today when > they migrate from one country to another (many US immigrants did > this, too) or when they change their religion (remember Cassius Clay?). > > Nevertheless I can understand that you are bound by the requirements > of your employer. > > Regards, > > Steve (that's the name by which I was called when I was a youngster)
On Fri, May 25, 2012 at 3:51 AM, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 05/24/2012 02:42 PM, Stefan Weil wrote: >> >> Am 24.05.2012 20:36, schrieb Anthony Liguori: >>> >>> On 05/24/2012 01:12 PM, Stefan Weil wrote: >>>> >>>> This discussion looks strange for me. >>> >>> >>> I'm not going to commit patches with a Signed-off-by if I know the name >>> is an >>> alias. >>> >>> DCO requires the use of a real name. DCO is an important part of ensuring >>> the >>> pedigree of a code base just like copyright licensing. >>> >>> Regards, >>> >>> Anthony Liguori >>> >> >> As I wrote in my previous mail, (personal) names are not absolute, >> but influenced by cultural facts. > > > I understand this. And this is why I generally don't challenge the names > people use with Signed-off-by. But when someone says that what they're > using is not their legal name and they will not share their legal name, I > have no choice but to not take patches from that person. I am a Windows researcher and not familiar with Linux, Git, GNU, etc. I can't make a big contribution, so I don't care if applying my patch. I just hope the bugs will be fixed ASAP. I had pointed out which commit introduced this bug. I thought this obvious issue would be found out easily by diff. But at the first time, Avi didn't reply me even if he was active until now. > Regards, > > Anthony Liguori > > >> DCO was something written by >> lawyers from the western culture where individual personal names >> exist since some time and where these names are usually >> more or less constant during the life of a person. AFAIK, >> DCO was introduced for legal reasons (SCO => DCO). >> >> It would be interesting to know how many names used in Linux >> commits since DCO are faked real names. Without enforcing >> certified signatures, enforcing "real names" (that's names which >> look like some name you know) does not really ensure something. >> Nor does the requirement of a valid mail address. >> It is only an alibi pedigree. >> >> Even in America and Europe, the concept of a "real name" is >> rather new (only some hundred years old), and there still exist cultures >> without personal names. See http://en.wikipedia.org/wiki/Personal_name >> for more information. People change their name even today when >> they migrate from one country to another (many US immigrants did >> this, too) or when they change their religion (remember Cassius Clay?). >> >> Nevertheless I can understand that you are bound by the requirements >> of your employer. >> >> Regards, >> >> Steve (that's the name by which I was called when I was a youngster) > > > > >
diff --git a/exec.c b/exec.c index 0607c9b..ad99476 100644 --- a/exec.c +++ b/exec.c @@ -1475,7 +1475,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr) static void breakpoint_invalidate(CPUArchState *env, target_ulong pc) { - tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc)); + tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc) + | (pc & ~TARGET_PAGE_MASK)); } #endif #endif /* TARGET_HAS_ICE */
This breakage was introduced by the commit "memory: make phys_page_find() return an unadjusted". Signed-off-by: TeLeMan <geleman@gmail.com> --- exec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)