qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
@ 2013-11-06 16:22 Luiz Capitulino
  2013-11-06 16:26 ` Paolo Bonzini
  2013-11-06 17:39 ` Paolo Bonzini
  0 siblings, 2 replies; 13+ messages in thread
From: Luiz Capitulino @ 2013-11-06 16:22 UTC (permalink / raw)
  To: marcel.a; +Cc: qemu-devel, mst

1. Run qemu with gdb server support

   # qemu [...] -s -S

2. Connect gdb and try to set a breakpoint

   $ gdb /path/to/vmlinux
   (gdb) target remote:1234
   (gdb) b secondary_startup_64

3. On qemu terminal

qemu-qmp: /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.
Aborted (core dumped)
   
According to bisect the culprit is:

commit a53ae8e934cd54686875b5bcfc2f434244ee55d6
Author: Marcel Apfelbaum <marcel.a@redhat.com>
Date:   Mon Sep 16 11:21:16 2013 +0300

    hw/pci: partially handle pci master abort

Backtrace:

#0  0x00007fd7882c2a19 in raise () from /lib64/libc.so.6
#1  0x00007fd7882c4128 in abort () from /lib64/libc.so.6
#2  0x00007fd7882bb986 in __assert_fail_base () from /lib64/libc.so.6
#3  0x00007fd7882bba32 in __assert_fail () from /lib64/libc.so.6
#4  0x00007fd78b7402ff in int128_get64 (a=...)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22
#5  address_space_translate_internal (d=<optimized out>, addr=18446744067283878160, 
    xlat=0x7fff7c13f498, plen=0x7fff7c13f530, resolve_subpage=<optimized out>)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/exec.c:263
#6  0x00007fd78b740d6c in address_space_translate (as=<optimized out>, 
    as@entry=0x7fd78c0ad4c0 <address_space_memory>, addr=addr@entry=18446744071578845456, 
    xlat=xlat@entry=0x7fff7c13f540, plen=plen@entry=0x7fff7c13f530, is_write=is_write@entry=false)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/exec.c:277
#7  0x00007fd78b742dc7 in address_space_rw (as=as@entry=0x7fd78c0ad4c0 <address_space_memory>, 
    addr=18446744071578845456, buf=buf@entry=0x7fff7c140620 "", len=len@entry=18, 
    is_write=is_write@entry=false) at /home/lcapitulino/work/src/upstream/qmp-unstable/exec.c:1883
#8  0x00007fd78b744ac1 in cpu_physical_memory_rw (is_write=0, len=18, buf=0x7fff7c140620 "", 
    addr=<optimized out>) at /home/lcapitulino/work/src/upstream/qmp-unstable/exec.c:1978
#9  cpu_memory_rw_debug (cpu=0x7fd78d63e320, addr=18446744071578845456, buf=0x7fff7c140620 "", 
    len=<optimized out>, is_write=0) at /home/lcapitulino/work/src/upstream/qmp-unstable/exec.c:2573
#10 0x00007fd78b75594a in target_memory_rw_debug (is_write=false, len=18, buf=0x7fff7c140620 "", 
    addr=18446744071578845456, cpu=0x7fd78d63e320)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/gdbstub.c:52
#11 gdb_handle_packet (s=s@entry=0x7fd78d6a6350, 
    line_buf=line_buf@entry=0x7fd78d6a636c "mffffffff81000110,12")
    at /home/lcapitulino/work/src/upstream/qmp-unstable/gdbstub.c:928
#12 0x00007fd78b7563f8 in gdb_read_byte (ch=55, s=0x7fd78d6a6350)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/gdbstub.c:1402
#13 gdb_chr_receive (opaque=<optimized out>, buf=<optimized out>, size=<optimized out>)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/gdbstub.c:1618
#14 0x00007fd78b6ef489 in qemu_chr_be_write (len=<optimized out>, 
    buf=0x7fff7c141740 "$mffffffff81000110,12#b7s+;xmlRegisters=i386;qRelocInsn+#b5|\377\177", 
    s=0x7fd78d9202c0) at /home/lcapitulino/work/src/upstream/qmp-unstable/qemu-char.c:165
#15 tcp_chr_read (chan=<optimized out>, cond=<optimized out>, opaque=0x7fd78d9202c0)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/qemu-char.c:2487
#16 0x00007fd78ac02e06 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
#17 0x00007fd78b6c01e8 in glib_pollfds_poll ()
    at /home/lcapitulino/work/src/upstream/qmp-unstable/main-loop.c:189
#18 os_host_main_loop_wait (timeout=<optimized out>)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/main-loop.c:234
#19 main_loop_wait (nonblocking=<optimized out>)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/main-loop.c:483
#20 0x00007fd78b597418 in main_loop () at /home/lcapitulino/work/src/upstream/qmp-unstable/vl.c:2014
#21 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
    at /home/lcapitulino/work/src/upstream/qmp-unstable/vl.c:4362

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 16:22 [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected) Luiz Capitulino
@ 2013-11-06 16:26 ` Paolo Bonzini
  2013-11-06 16:29   ` Luiz Capitulino
  2013-11-06 16:33   ` Michael S. Tsirkin
  2013-11-06 17:39 ` Paolo Bonzini
  1 sibling, 2 replies; 13+ messages in thread
From: Paolo Bonzini @ 2013-11-06 16:26 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: mst, qemu-devel, marcel.a

Il 06/11/2013 17:22, Luiz Capitulino ha scritto:
> 1. Run qemu with gdb server support
> 
>    # qemu [...] -s -S
> 
> 2. Connect gdb and try to set a breakpoint
> 
>    $ gdb /path/to/vmlinux
>    (gdb) target remote:1234
>    (gdb) b secondary_startup_64
> 
> 3. On qemu terminal
> 
> qemu-qmp: /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.
> Aborted (core dumped)

Do you have "exec: limit system memory size"?

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 16:26 ` Paolo Bonzini
@ 2013-11-06 16:29   ` Luiz Capitulino
  2013-11-06 16:33   ` Michael S. Tsirkin
  1 sibling, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2013-11-06 16:29 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: mst, qemu-devel, marcel.a

On Wed, 06 Nov 2013 17:26:32 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 06/11/2013 17:22, Luiz Capitulino ha scritto:
> > 1. Run qemu with gdb server support
> > 
> >    # qemu [...] -s -S
> > 
> > 2. Connect gdb and try to set a breakpoint
> > 
> >    $ gdb /path/to/vmlinux
> >    (gdb) target remote:1234
> >    (gdb) b secondary_startup_64
> > 
> > 3. On qemu terminal
> > 
> > qemu-qmp: /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.
> > Aborted (core dumped)
> 
> Do you have "exec: limit system memory size"?

Yes, my HEAD is:

commit a30b377e0a921bf93349dc4adb94580a3bec7ea4
Merge: c905c50 80bbaee
Author: Anthony Liguori <aliguori@amazon.com>
Date:   Tue Nov 5 10:33:32 2013 -0800

    Merge remote-tracking branch 'afaerber/tags/qom-devices-for-anthony' into staging

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 16:33   ` Michael S. Tsirkin
@ 2013-11-06 16:30     ` Marcel Apfelbaum
  0 siblings, 0 replies; 13+ messages in thread
From: Marcel Apfelbaum @ 2013-11-06 16:30 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Paolo Bonzini, qemu-devel, Luiz Capitulino

On Wed, 2013-11-06 at 18:33 +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 06, 2013 at 05:26:32PM +0100, Paolo Bonzini wrote:
> > Il 06/11/2013 17:22, Luiz Capitulino ha scritto:
> > > 1. Run qemu with gdb server support
> > > 
> > >    # qemu [...] -s -S
> > > 
> > > 2. Connect gdb and try to set a breakpoint
> > > 
> > >    $ gdb /path/to/vmlinux
> > >    (gdb) target remote:1234
> > >    (gdb) b secondary_startup_64
> > > 
> > > 3. On qemu terminal
> > > 
> > > qemu-qmp: /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.
> > > Aborted (core dumped)
> > 
> > Do you have "exec: limit system memory size"?
> > 
> > Paolo
> 
> Happens anyway :) Debugging.

I'll also assist tonight!
Marcel 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 16:26 ` Paolo Bonzini
  2013-11-06 16:29   ` Luiz Capitulino
@ 2013-11-06 16:33   ` Michael S. Tsirkin
  2013-11-06 16:30     ` Marcel Apfelbaum
  1 sibling, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2013-11-06 16:33 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: marcel.a, qemu-devel, Luiz Capitulino

On Wed, Nov 06, 2013 at 05:26:32PM +0100, Paolo Bonzini wrote:
> Il 06/11/2013 17:22, Luiz Capitulino ha scritto:
> > 1. Run qemu with gdb server support
> > 
> >    # qemu [...] -s -S
> > 
> > 2. Connect gdb and try to set a breakpoint
> > 
> >    $ gdb /path/to/vmlinux
> >    (gdb) target remote:1234
> >    (gdb) b secondary_startup_64
> > 
> > 3. On qemu terminal
> > 
> > qemu-qmp: /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.
> > Aborted (core dumped)
> 
> Do you have "exec: limit system memory size"?
> 
> Paolo

Happens anyway :) Debugging.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 16:22 [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected) Luiz Capitulino
  2013-11-06 16:26 ` Paolo Bonzini
@ 2013-11-06 17:39 ` Paolo Bonzini
  2013-11-06 17:48   ` Michael S. Tsirkin
  2013-11-06 18:36   ` Luiz Capitulino
  1 sibling, 2 replies; 13+ messages in thread
From: Paolo Bonzini @ 2013-11-06 17:39 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: mst, qemu-devel, marcel.a

Il 06/11/2013 17:22, Luiz Capitulino ha scritto:
> 1. Run qemu with gdb server support
> 
>    # qemu [...] -s -S
> 
> 2. Connect gdb and try to set a breakpoint
> 
>    $ gdb /path/to/vmlinux
>    (gdb) target remote:1234
>    (gdb) b secondary_startup_64

(Note that this doesn't make much sense until the kernel has been loaded
into memory.  You probably want hbreak instead).

> 3. On qemu terminal
> 
> qemu-qmp: /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.
> Aborted (core dumped)
>    
> According to bisect the culprit is:
> 
> commit a53ae8e934cd54686875b5bcfc2f434244ee55d6
> Author: Marcel Apfelbaum <marcel.a@redhat.com>
> Date:   Mon Sep 16 11:21:16 2013 +0300
> 
>     hw/pci: partially handle pci master abort

I couldn't get quite the same reproducer, mine was:

$ gdb
(gdb) set arch  i386:x86-64
The target architecture is assumed to be i386:x86-64
(gdb) target remote localhost:1234
Remote debugging using localhost:1234
<bang>

The problem is that gdb attempts to read a few bytes from address
0xffffffffffffffe6 to 0xffffffffffffffff inclusive.

The region it gets is the (newly introduced) master abort region, which
is as big as the PCI address space (see pci_bus_init).  Due to a typo
that's only 2^63-1, not 2^64.  But we get it anyway because
phys_page_find ignores the upper bits of the physical address.  In
address_space_translate_internal then

    diff = int128_sub(section->mr->size, int128_make64(addr));
    *plen = int128_get64(int128_min(diff, int128_make64(*plen)));

diff becomes negative, and int128_get64 booms.

Will send as a proper patch tomorrow... can you give your Tested-by?


diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b3d94bd..68901c3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -114,7 +114,7 @@ static void pc_init1(QEMUMachineInitArgs *args,

     if (pci_enabled) {
         pci_memory = g_new(MemoryRegion, 1);
-        memory_region_init(pci_memory, NULL, "pci", INT64_MAX);
+        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
     } else {
         pci_memory = NULL;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 2e315f7..c9a03fc 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -101,7 +101,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     /* pci enabled */
     if (pci_enabled) {
         pci_memory = g_new(MemoryRegion, 1);
-        memory_region_init(pci_memory, NULL, "pci", INT64_MAX);
+        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
     } else {
         pci_memory = NULL;

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 17:39 ` Paolo Bonzini
@ 2013-11-06 17:48   ` Michael S. Tsirkin
  2013-11-06 17:50     ` Paolo Bonzini
  2013-11-06 18:36   ` Luiz Capitulino
  1 sibling, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2013-11-06 17:48 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: marcel.a, qemu-devel, Luiz Capitulino

On Wed, Nov 06, 2013 at 06:39:42PM +0100, Paolo Bonzini wrote:
> Il 06/11/2013 17:22, Luiz Capitulino ha scritto:
> > 1. Run qemu with gdb server support
> > 
> >    # qemu [...] -s -S
> > 
> > 2. Connect gdb and try to set a breakpoint
> > 
> >    $ gdb /path/to/vmlinux
> >    (gdb) target remote:1234
> >    (gdb) b secondary_startup_64
> 
> (Note that this doesn't make much sense until the kernel has been loaded
> into memory.  You probably want hbreak instead).
> 
> > 3. On qemu terminal
> > 
> > qemu-qmp: /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.
> > Aborted (core dumped)
> >    
> > According to bisect the culprit is:
> > 
> > commit a53ae8e934cd54686875b5bcfc2f434244ee55d6
> > Author: Marcel Apfelbaum <marcel.a@redhat.com>
> > Date:   Mon Sep 16 11:21:16 2013 +0300
> > 
> >     hw/pci: partially handle pci master abort
> 
> I couldn't get quite the same reproducer, mine was:
> 
> $ gdb
> (gdb) set arch  i386:x86-64
> The target architecture is assumed to be i386:x86-64
> (gdb) target remote localhost:1234
> Remote debugging using localhost:1234
> <bang>
> 
> The problem is that gdb attempts to read a few bytes from address
> 0xffffffffffffffe6 to 0xffffffffffffffff inclusive.
> 
> The region it gets is the (newly introduced) master abort region, which
> is as big as the PCI address space (see pci_bus_init).  Due to a typo
> that's only 2^63-1, not 2^64.  But we get it anyway because
> phys_page_find ignores the upper bits of the physical address.  In
> address_space_translate_internal then
> 
>     diff = int128_sub(section->mr->size, int128_make64(addr));
>     *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
> 
> diff becomes negative, and int128_get64 booms.
> 
> Will send as a proper patch tomorrow... can you give your Tested-by?


This just makes the symproms go away.
The real bug is exec ignores high address bits during page
lookups. It should fail on invalid address not access
a random page.
I'll send a patch.



> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index b3d94bd..68901c3 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -114,7 +114,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> 
>      if (pci_enabled) {
>          pci_memory = g_new(MemoryRegion, 1);
> -        memory_region_init(pci_memory, NULL, "pci", INT64_MAX);
> +        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>          rom_memory = pci_memory;
>      } else {
>          pci_memory = NULL;
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 2e315f7..c9a03fc 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -101,7 +101,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>      /* pci enabled */
>      if (pci_enabled) {
>          pci_memory = g_new(MemoryRegion, 1);
> -        memory_region_init(pci_memory, NULL, "pci", INT64_MAX);
> +        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>          rom_memory = pci_memory;
>      } else {
>          pci_memory = NULL;
> 


This is also ugly and will be broken on actual 64 bit systems
(not x86). Generally INT64_MAX does not make sense at all.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 17:48   ` Michael S. Tsirkin
@ 2013-11-06 17:50     ` Paolo Bonzini
  2013-11-06 18:39       ` Michael S. Tsirkin
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2013-11-06 17:50 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: marcel.a, qemu-devel, Luiz Capitulino

Il 06/11/2013 18:48, Michael S. Tsirkin ha scritto:
> This just makes the symproms go away.

That's correct.

> The real bug is exec ignores high address bits during page
> lookups. It should fail on invalid address not access
> a random page.
> I'll send a patch.

The real real bug is that all address spaces should be 2^64, which you
said you consider too intrusive a patch.  I don't feel confident
changing phys_page_find, even if it's just 2 lines.

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 17:39 ` Paolo Bonzini
  2013-11-06 17:48   ` Michael S. Tsirkin
@ 2013-11-06 18:36   ` Luiz Capitulino
  2013-11-06 21:11     ` Paolo Bonzini
  1 sibling, 1 reply; 13+ messages in thread
From: Luiz Capitulino @ 2013-11-06 18:36 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: mst, qemu-devel, marcel.a

On Wed, 06 Nov 2013 18:39:42 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 06/11/2013 17:22, Luiz Capitulino ha scritto:
> > 1. Run qemu with gdb server support
> > 
> >    # qemu [...] -s -S
> > 
> > 2. Connect gdb and try to set a breakpoint
> > 
> >    $ gdb /path/to/vmlinux
> >    (gdb) target remote:1234
> >    (gdb) b secondary_startup_64
> 
> (Note that this doesn't make much sense until the kernel has been loaded
> into memory.  You probably want hbreak instead).

hbreak didn't work either, gdb doesn't stop at the breakpoint. I tried to
test this with another random function and got a "Remote 'g' packet
reply is too long" (which seems to be yet another different problem).

> > 3. On qemu terminal
> > 
> > qemu-qmp: /home/lcapitulino/work/src/upstream/qmp-unstable/include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.
> > Aborted (core dumped)
> >    
> > According to bisect the culprit is:
> > 
> > commit a53ae8e934cd54686875b5bcfc2f434244ee55d6
> > Author: Marcel Apfelbaum <marcel.a@redhat.com>
> > Date:   Mon Sep 16 11:21:16 2013 +0300
> > 
> >     hw/pci: partially handle pci master abort
> 
> I couldn't get quite the same reproducer, mine was:
> 
> $ gdb
> (gdb) set arch  i386:x86-64
> The target architecture is assumed to be i386:x86-64
> (gdb) target remote localhost:1234
> Remote debugging using localhost:1234
> <bang>
> 
> The problem is that gdb attempts to read a few bytes from address
> 0xffffffffffffffe6 to 0xffffffffffffffff inclusive.
> 
> The region it gets is the (newly introduced) master abort region, which
> is as big as the PCI address space (see pci_bus_init).  Due to a typo
> that's only 2^63-1, not 2^64.  But we get it anyway because
> phys_page_find ignores the upper bits of the physical address.  In
> address_space_translate_internal then
> 
>     diff = int128_sub(section->mr->size, int128_make64(addr));
>     *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
> 
> diff becomes negative, and int128_get64 booms.
> 
> Will send as a proper patch tomorrow... can you give your Tested-by?

Tested-by: Luiz Capitulino <lcapitulino@redhat.com>

> 
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index b3d94bd..68901c3 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -114,7 +114,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> 
>      if (pci_enabled) {
>          pci_memory = g_new(MemoryRegion, 1);
> -        memory_region_init(pci_memory, NULL, "pci", INT64_MAX);
> +        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>          rom_memory = pci_memory;
>      } else {
>          pci_memory = NULL;
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 2e315f7..c9a03fc 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -101,7 +101,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>      /* pci enabled */
>      if (pci_enabled) {
>          pci_memory = g_new(MemoryRegion, 1);
> -        memory_region_init(pci_memory, NULL, "pci", INT64_MAX);
> +        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>          rom_memory = pci_memory;
>      } else {
>          pci_memory = NULL;
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 17:50     ` Paolo Bonzini
@ 2013-11-06 18:39       ` Michael S. Tsirkin
  2013-11-06 21:13         ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2013-11-06 18:39 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: marcel.a, qemu-devel, Luiz Capitulino

On Wed, Nov 06, 2013 at 06:50:05PM +0100, Paolo Bonzini wrote:
> Il 06/11/2013 18:48, Michael S. Tsirkin ha scritto:
> > This just makes the symproms go away.
> 
> That's correct.
> 
> > The real bug is exec ignores high address bits during page
> > lookups. It should fail on invalid address not access
> > a random page.
> > I'll send a patch.
> 
> The real real bug is that all address spaces should be 2^64, which you
> said you consider too intrusive a patch.

Because this will affect performance in unpredicatable way.
We can't make such changes in 1.7 IMHO:
it would need much more than just a quick "works for me".

>  I don't feel confident
> changing phys_page_find, even if it's just 2 lines.
> 
> Paolo

Well it's *obviously* broken if address is outside target address
space.
Take a look at the patch first, then argue.


-- 
MST

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 18:36   ` Luiz Capitulino
@ 2013-11-06 21:11     ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2013-11-06 21:11 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: marcel.a, qemu-devel, mst

Il 06/11/2013 19:36, Luiz Capitulino ha scritto:
> On Wed, 06 Nov 2013 18:39:42 +0100
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Il 06/11/2013 17:22, Luiz Capitulino ha scritto:
>>> 1. Run qemu with gdb server support
>>>
>>>    # qemu [...] -s -S
>>>
>>> 2. Connect gdb and try to set a breakpoint
>>>
>>>    $ gdb /path/to/vmlinux
>>>    (gdb) target remote:1234
>>>    (gdb) b secondary_startup_64
>>
>> (Note that this doesn't make much sense until the kernel has been loaded
>> into memory.  You probably want hbreak instead).
> 
> hbreak didn't work either, gdb doesn't stop at the breakpoint. I tried to
> test this with another random function and got a "Remote 'g' packet
> reply is too long" (which seems to be yet another different problem).

Yeah, that's very messy and it would nice to have a fix for it, but I
don't know enough about gdb to say whether it's fixable.

It happens when the processor switches from 32 to 64-bit under gdb's
feet.  The solution is typically to do "set arch  i386:x86-64" before
running the guest with "c" if you know the breakpoint will happen in
64-bit mode.

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 18:39       ` Michael S. Tsirkin
@ 2013-11-06 21:13         ` Paolo Bonzini
  2013-11-06 21:36           ` Michael S. Tsirkin
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2013-11-06 21:13 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Luiz Capitulino, qemu-devel, marcel.a

Il 06/11/2013 19:39, Michael S. Tsirkin ha scritto:
> Because this will affect performance in unpredicatable way.

It's not really unpredictable.  It can be easily unit-tested, and anyway
the targets with 64-bit address spaces don't have any particular
performance problem.

> We can't make such changes in 1.7 IMHO:
> it would need much more than just a quick "works for me".

I can say the same about phys_page_find.  It's been obviously broken for
years and nobody ever cared, it cannot be super-urgent now to fix it.

Paolo

>> >  I don't feel confident
>> > changing phys_page_find, even if it's just 2 lines.
>> > 
>> > Paolo
> Well it's *obviously* broken if address is outside target address
> space.
> Take a look at the patch first, then argue.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)
  2013-11-06 21:13         ` Paolo Bonzini
@ 2013-11-06 21:36           ` Michael S. Tsirkin
  0 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2013-11-06 21:36 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Luiz Capitulino, qemu-devel, marcel.a

On Wed, Nov 06, 2013 at 10:13:16PM +0100, Paolo Bonzini wrote:
> Il 06/11/2013 19:39, Michael S. Tsirkin ha scritto:
> > Because this will affect performance in unpredicatable way.
> 
> It's not really unpredictable.  It can be easily unit-tested,

Go ahead, post, test, I'm not stopping you.

> and anyway
> the targets with 64-bit address spaces don't have any particular
> performance problem.

That's only s390x and they happen not to use IO for anything,
instead they do hypercalls. So hard to tell.

> > We can't make such changes in 1.7 IMHO:
> > it would need much more than just a quick "works for me".
> 
> I can say the same about phys_page_find.

Why? It's *obvious* this code is not designed to work with
addresses > target address space.
You doubt that?
What is the worry here?

>  It's been obviously broken for
> years and nobody ever cared, it cannot be super-urgent now to fix it.
> 
> Paolo

Well we added a bunch of code and now it is failing.

> >> >  I don't feel confident
> >> > changing phys_page_find, even if it's just 2 lines.
> >> > 
> >> > Paolo
> > Well it's *obviously* broken if address is outside target address
> > space.
> > Take a look at the patch first, then argue.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-11-06 21:33 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-06 16:22 [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected) Luiz Capitulino
2013-11-06 16:26 ` Paolo Bonzini
2013-11-06 16:29   ` Luiz Capitulino
2013-11-06 16:33   ` Michael S. Tsirkin
2013-11-06 16:30     ` Marcel Apfelbaum
2013-11-06 17:39 ` Paolo Bonzini
2013-11-06 17:48   ` Michael S. Tsirkin
2013-11-06 17:50     ` Paolo Bonzini
2013-11-06 18:39       ` Michael S. Tsirkin
2013-11-06 21:13         ` Paolo Bonzini
2013-11-06 21:36           ` Michael S. Tsirkin
2013-11-06 18:36   ` Luiz Capitulino
2013-11-06 21:11     ` Paolo Bonzini

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