[Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

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?

-- 
--
SUN OF A BEACH

[-- Attachment #2: Type: text/html, Size: 1146 bytes --]

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Andreas Färber]

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 */

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

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

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

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

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

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

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

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
>

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2560 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[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.

>
> /-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
>

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Andreas Färber]

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

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Blue Swirl]

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Anthony Liguori]

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[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>


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Stefan Weil]

[-- Attachment #1: Type: text/plain, Size: 2713 bytes --]

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.


[-- Attachment #2: Type: text/html, Size: 4796 bytes --]

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Anthony Liguori]

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Stefan Weil]

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)

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Anthony Liguori]

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)

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

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)
>
>
>
>
>