Re: [Qemu-devel] BUG: QEMU aborts when setting breakpoint in gdb (bisected)

["Michael S. Tsirkin" <mst@redhat.com>]

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.