[Qemu-devel] [memory] abort with head a8170e5

[Qemu-devel] [memory] abort with head a8170e5

[Richard Henderson]

qemu-system-sparc64: /home/rth/work/qemu/qemu/memory.c:1022: memory_region_destroy: Assertion `memory_region_transaction_depth == 0' failed.

Program received signal SIGABRT, Aborted.
0x00007ffff5234925 in raise () from /lib64/libc.so.6
(gdb) where
#0  0x00007ffff5234925 in raise () from /lib64/libc.so.6
#1  0x00007ffff52360d8 in abort () from /lib64/libc.so.6
#2  0x00007ffff522d6a2 in __assert_fail_base () from /lib64/libc.so.6
#3  0x00007ffff522d752 in __assert_fail () from /lib64/libc.so.6
#4  0x000055555576ebe4 in memory_region_destroy (mr=0x555556a76f60)
    at /home/rth/work/qemu/qemu/memory.c:1022
#5  0x0000555555674729 in pci_bridge_cleanup_alias (
    parent_space=<optimized out>, alias=0x555556a76f60)
    at /home/rth/work/qemu/qemu/hw/pci_bridge.c:158
#6  pci_bridge_region_cleanup (br=0x555556a75d30)
    at /home/rth/work/qemu/qemu/hw/pci_bridge.c:190
#7  0x0000555555674ccb in pci_bridge_update_mappings (br=0x555556a75d30)
    at /home/rth/work/qemu/qemu/hw/pci_bridge.c:203
#8  pci_bridge_write_config (d=0x555556a75d30, address=<optimized out>, 
    val=<optimized out>, len=<optimized out>)
    at /home/rth/work/qemu/qemu/hw/pci_bridge.c:226
#9  0x000055555576b072 in access_with_adjusted_size (addr=addr@entry=2052, 
    value=value@entry=0x7fffedaee890, size=size@entry=2, 
    access_size_min=<optimized out>, access_size_max=<optimized out>, 
    access=access@entry=0x55555576b690 <memory_region_write_accessor>, 
    opaque=opaque@entry=0x555556a65a38)
    at /home/rth/work/qemu/qemu/memory.c:363
#10 0x0000555555770183 in memory_region_dispatch_write (size=2, data=768, addr=
    2052, mr=0x555556a65a38) at /home/rth/work/qemu/qemu/memory.c:914
#11 io_mem_write (mr=0x555556a65a38, addr=2052, val=<optimized out>, size=2)
    at /home/rth/work/qemu/qemu/memory.c:1570
#12 0x00007ffff011cd3e in code_gen_buffer ()

This can be seen with the distributed OpenBIOS, i.e. no special options needed:

  ./sparc64-softmmu/qemu-system-sparc64


r~

Re: [Qemu-devel] [memory] abort with head a8170e5

[Aurelien Jarno]

On Wed, Oct 24, 2012 at 09:15:32AM +1000, Richard Henderson wrote:
> qemu-system-sparc64: /home/rth/work/qemu/qemu/memory.c:1022: memory_region_destroy: Assertion `memory_region_transaction_depth == 0' failed.
> 
> Program received signal SIGABRT, Aborted.
> 0x00007ffff5234925 in raise () from /lib64/libc.so.6
> (gdb) where
> #0  0x00007ffff5234925 in raise () from /lib64/libc.so.6
> #1  0x00007ffff52360d8 in abort () from /lib64/libc.so.6
> #2  0x00007ffff522d6a2 in __assert_fail_base () from /lib64/libc.so.6
> #3  0x00007ffff522d752 in __assert_fail () from /lib64/libc.so.6
> #4  0x000055555576ebe4 in memory_region_destroy (mr=0x555556a76f60)
>     at /home/rth/work/qemu/qemu/memory.c:1022
> #5  0x0000555555674729 in pci_bridge_cleanup_alias (
>     parent_space=<optimized out>, alias=0x555556a76f60)
>     at /home/rth/work/qemu/qemu/hw/pci_bridge.c:158
> #6  pci_bridge_region_cleanup (br=0x555556a75d30)
>     at /home/rth/work/qemu/qemu/hw/pci_bridge.c:190
> #7  0x0000555555674ccb in pci_bridge_update_mappings (br=0x555556a75d30)
>     at /home/rth/work/qemu/qemu/hw/pci_bridge.c:203
> #8  pci_bridge_write_config (d=0x555556a75d30, address=<optimized out>, 
>     val=<optimized out>, len=<optimized out>)
>     at /home/rth/work/qemu/qemu/hw/pci_bridge.c:226
> #9  0x000055555576b072 in access_with_adjusted_size (addr=addr@entry=2052, 
>     value=value@entry=0x7fffedaee890, size=size@entry=2, 
>     access_size_min=<optimized out>, access_size_max=<optimized out>, 
>     access=access@entry=0x55555576b690 <memory_region_write_accessor>, 
>     opaque=opaque@entry=0x555556a65a38)
>     at /home/rth/work/qemu/qemu/memory.c:363
> #10 0x0000555555770183 in memory_region_dispatch_write (size=2, data=768, addr=
>     2052, mr=0x555556a65a38) at /home/rth/work/qemu/qemu/memory.c:914
> #11 io_mem_write (mr=0x555556a65a38, addr=2052, val=<optimized out>, size=2)
>     at /home/rth/work/qemu/qemu/memory.c:1570
> #12 0x00007ffff011cd3e in code_gen_buffer ()
> 
> This can be seen with the distributed OpenBIOS, i.e. no special options needed:
> 
>   ./sparc64-softmmu/qemu-system-sparc64
> 

mips is also broken but by commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d:

| [    0.436000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
| Segmentation fault (core dumped)

With gdb:

| Program terminated with signal 11, Segmentation fault.
| #0  phys_page_set_level (lp=0x7f4e12862db0, lp@entry=0x7f4e12851cf0, index=index@entry=0x7f4e012af480, nb=nb@entry=0x7f4e012af488, leaf=leaf@entry=45, level=level@entry=0) at /home/aurel32/qemu/exec.c:440
| 440                 lp->is_leaf = true;
| (gdb) bt
| #0  phys_page_set_level (lp=0x7f4e12862db0, lp@entry=0x7f4e12851cf0, index=index@entry=0x7f4e012af480, nb=nb@entry=0x7f4e012af488, leaf=leaf@entry=45, level=level@entry=0) at /home/aurel32/qemu/exec.c:440
| #1  0x00007f4e10f33a10 in phys_page_set_level (lp=0x7f4e12851cf0, lp@entry=0x7f4e12851470, index=index@entry=0x7f4e012af480, nb=nb@entry=0x7f4e012af488, leaf=leaf@entry=45, level=level@entry=1)
|     at /home/aurel32/qemu/exec.c:445
| #2  0x00007f4e10f33a10 in phys_page_set_level (lp=0x7f4e12851470, lp@entry=0x7f4e124ffb50, index=index@entry=0x7f4e012af480, nb=nb@entry=0x7f4e012af488, leaf=45, level=level@entry=2)
|     at /home/aurel32/qemu/exec.c:445
| #3  0x00007f4e10f3477f in phys_page_set (leaf=<optimized out>, nb=16, index=65696, d=0x7f4e124ffb50) at /home/aurel32/qemu/exec.c:458
| #4  register_multipage (section=0x7f4e012af490, d=0x7f4e124ffb50) at /home/aurel32/qemu/exec.c:2263
| #5  mem_add (listener=0x7f4e124ffb58, section=<optimized out>) at /home/aurel32/qemu/exec.c:2289
| #6  0x00007f4e10f69a3c in address_space_update_topology_pass (as=as@entry=0x7f4e126201c8, adding=adding@entry=true, old_view=..., new_view=...) at /home/aurel32/qemu/memory.c:710
| #7  0x00007f4e10f6a458 in address_space_update_topology (as=0x7f4e126201c8) at /home/aurel32/qemu/memory.c:725
| #8  memory_region_transaction_commit () at /home/aurel32/qemu/memory.c:748
| #9  0x00007f4e10e5eeff in pci_default_write_config (d=0x7f4e1261ffb0, addr=4, val=0, l=4) at hw/pci.c:1075
| #10 0x00007f4e10f67df2 in access_with_adjusted_size (addr=addr@entry=3324, value=value@entry=0x7f4e012af8a0, size=size@entry=4, access_size_min=<optimized out>, access_size_max=<optimized out>,
|     access=access@entry=0x7f4e10f68410 <memory_region_write_accessor>, opaque=opaque@entry=0x7f4e124f2ba8) at /home/aurel32/qemu/memory.c:363
| #11 0x00007f4e10f6cda3 in memory_region_dispatch_write (size=4, data=41943045, addr=3324, mr=0x7f4e124f2ba8) at /home/aurel32/qemu/memory.c:914
| #12 io_mem_write (mr=0x7f4e124f2ba8, addr=3324, val=<optimized out>, size=4) at /home/aurel32/qemu/memory.c:1567
| #13 0x00000000415a4be0 in code_gen_buffer ()
| #14 0x00007f4e10f2e811 in cpu_mips_exec (env=0x7f4e12840ed0, env@entry=0x7f4e124d98c8) at /home/aurel32/qemu/cpu-exec.c:601
| #15 0x00007f4e10f2fbc3 in tcg_cpu_exec (env=0x7f4e124d98c8) at /home/aurel32/qemu/cpus.c:1109
| #16 tcg_exec_all () at /home/aurel32/qemu/cpus.c:1141
| #17 qemu_tcg_cpu_thread_fn (arg=<optimized out>) at /home/aurel32/qemu/cpus.c:836
| #18 0x00007f4e0c2a3b50 in start_thread (arg=<optimized out>) at pthread_create.c:304
| #19 0x00007f4e0bfee70d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
| #20 0x0000000000000000 in ?? ()



-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH] pci: avoid destroying bridge address space windows in a transaction

[Avi Kivity]

Calling memory_region_destroy() in a transaction is illegal (and aborts),
as until the transaction is committed, the region remains live.

Fix by moving destruction until after the transaction commits.  This requires
having an extra set of regions, so the new and old regions can coexist.

Signed-off-by: Avi Kivity <avi@redhat.com>
---

Please review, and check if this patch fixes things for you.

 hw/pci_bridge.c    | 50 ++++++++++++++++++++++++++++----------------------
 hw/pci_internals.h | 24 ++++++++++++++++--------
 2 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 5c6455f..4680501 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -151,58 +151,63 @@ static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
     memory_region_add_subregion_overlap(parent_space, base, alias, 1);
 }
 
-static void pci_bridge_cleanup_alias(MemoryRegion *alias,
-                                     MemoryRegion *parent_space)
-{
-    memory_region_del_subregion(parent_space, alias);
-    memory_region_destroy(alias);
-}
-
-static void pci_bridge_region_init(PCIBridge *br)
+static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
 {
     PCIBus *parent = br->dev.bus;
+    PCIBridgeWindows *w = g_new(PCIBridgeWindows, 1);
     uint16_t cmd = pci_get_word(br->dev.config + PCI_COMMAND);
 
-    pci_bridge_init_alias(br, &br->alias_pref_mem,
+    pci_bridge_init_alias(br, &w->alias_pref_mem,
                           PCI_BASE_ADDRESS_MEM_PREFETCH,
                           "pci_bridge_pref_mem",
                           &br->address_space_mem,
                           parent->address_space_mem,
                           cmd & PCI_COMMAND_MEMORY);
-    pci_bridge_init_alias(br, &br->alias_mem,
+    pci_bridge_init_alias(br, &w->alias_mem,
                           PCI_BASE_ADDRESS_SPACE_MEMORY,
                           "pci_bridge_mem",
                           &br->address_space_mem,
                           parent->address_space_mem,
                           cmd & PCI_COMMAND_MEMORY);
-    pci_bridge_init_alias(br, &br->alias_io,
+    pci_bridge_init_alias(br, &w->alias_io,
                           PCI_BASE_ADDRESS_SPACE_IO,
                           "pci_bridge_io",
                           &br->address_space_io,
                           parent->address_space_io,
                           cmd & PCI_COMMAND_IO);
    /* TODO: optinal VGA and VGA palette snooping support. */
+
+    return w;
 }
 
-static void pci_bridge_region_cleanup(PCIBridge *br)
+static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
 {
     PCIBus *parent = br->dev.bus;
-    pci_bridge_cleanup_alias(&br->alias_io,
-                             parent->address_space_io);
-    pci_bridge_cleanup_alias(&br->alias_mem,
-                             parent->address_space_mem);
-    pci_bridge_cleanup_alias(&br->alias_pref_mem,
-                             parent->address_space_mem);
+
+    memory_region_del_subregion(parent->address_space_io, &w->alias_io);
+    memory_region_del_subregion(parent->address_space_mem, &w->alias_mem);
+    memory_region_del_subregion(parent->address_space_mem, &w->alias_pref_mem);
+}
+
+static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
+{
+    memory_region_destroy(&w->alias_io);
+    memory_region_destroy(&w->alias_mem);
+    memory_region_destroy(&w->alias_pref_mem);
+    g_free(w);
 }
 
 static void pci_bridge_update_mappings(PCIBridge *br)
 {
+    PCIBridgeWindows *w = br->windows;
+
     /* Make updates atomic to: handle the case of one VCPU updating the bridge
      * while another accesses an unaffected region. */
     memory_region_transaction_begin();
-    pci_bridge_region_cleanup(br);
-    pci_bridge_region_init(br);
+    pci_bridge_region_del(br, br->windows);
+    br->windows = pci_bridge_region_init(br);
     memory_region_transaction_commit();
+    pci_bridge_region_cleanup(br, w);
 }
 
 /* default write_config function for PCI-to-PCI bridge */
@@ -326,7 +331,7 @@ int pci_bridge_initfn(PCIDevice *dev)
     memory_region_init(&br->address_space_mem, "pci_bridge_pci", INT64_MAX);
     sec_bus->address_space_io = &br->address_space_io;
     memory_region_init(&br->address_space_io, "pci_bridge_io", 65536);
-    pci_bridge_region_init(br);
+    br->windows = pci_bridge_region_init(br);
     QLIST_INIT(&sec_bus->child);
     QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
     return 0;
@@ -338,7 +343,8 @@ void pci_bridge_exitfn(PCIDevice *pci_dev)
     PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
     assert(QLIST_EMPTY(&s->sec_bus.child));
     QLIST_REMOVE(&s->sec_bus, sibling);
-    pci_bridge_region_cleanup(s);
+    pci_bridge_region_del(s, s->windows);
+    pci_bridge_region_cleanup(s, s->windows);
     memory_region_destroy(&s->address_space_mem);
     memory_region_destroy(&s->address_space_io);
     /* qbus_free() is called automatically by qdev_free() */
diff --git a/hw/pci_internals.h b/hw/pci_internals.h
index c931b64..21d0ce6 100644
--- a/hw/pci_internals.h
+++ b/hw/pci_internals.h
@@ -40,6 +40,19 @@ struct PCIBus {
     int *irq_count;
 };
 
+typedef struct PCIBridgeWindows PCIBridgeWindows;
+
+/*
+ * Aliases for each of the address space windows that the bridge
+ * can forward. Mapped into the bridge's parent's address space,
+ * as subregions.
+ */
+struct PCIBridgeWindows {
+    MemoryRegion alias_pref_mem;
+    MemoryRegion alias_mem;
+    MemoryRegion alias_io;
+};
+
 struct PCIBridge {
     PCIDevice dev;
 
@@ -55,14 +68,9 @@ struct PCIBridge {
      */
     MemoryRegion address_space_mem;
     MemoryRegion address_space_io;
-    /*
-     * Aliases for each of the address space windows that the bridge
-     * can forward. Mapped into the bridge's parent's address space,
-     * as subregions.
-     */
-    MemoryRegion alias_pref_mem;
-    MemoryRegion alias_mem;
-    MemoryRegion alias_io;
+
+    PCIBridgeWindows *windows;
+
     pci_map_irq_fn map_irq;
     const char *bus_name;
 };
-- 
1.7.12

Re: [Qemu-devel] [memory] abort with head a8170e5

[Avi Kivity]

On 10/24/2012 04:00 PM, Aurelien Jarno wrote:
> 
> mips is also broken but by commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d:
> 
> | [    0.436000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
> | Segmentation fault (core dumped)
> 

How do you reproduce it?

Does this patch fix it for you?

From: Avi Kivity <avi@redhat.com>
Date: Thu, 11 Oct 2012 12:40:24 +0200
Subject: [PATCH] memory: limit sections in the radix tree to the actual
 address space size

The radix tree is statically sized to fit TARGET_PHYS_ADDR_SPACE_BITS.
If a larger memory region is registered, it will overflow.

Fix by limiting any section in the radix tree to the supported size.

This problem was not observed earlier since artificial regions (containers
and aliases) are eliminated by the memory core, leaving only device regions
which have reasonable sizes.  An IOMMU however cannot be eliminated by the
memory core, and may have an artificial size.

Signed-off-by: Avi Kivity <avi@redhat.com>

diff --git a/exec.c b/exec.c
index b0ed593..deee8ec 100644
--- a/exec.c
+++ b/exec.c
@@ -2280,10 +2280,23 @@ static void register_multipage(AddressSpaceDispatch *d, MemoryRegionSection *sec
                   section_index);
 }
 
+static MemoryRegionSection limit(MemoryRegionSection section)
+{
+    unsigned practical_as_bits = MIN(TARGET_PHYS_ADDR_SPACE_BITS, 62);
+    hwaddr as_limit;
+
+    as_limit = (hwaddr)1 << practical_as_bits;
+
+    section.size = MIN(section.offset_within_address_space + section.size, as_limit)
+                   - section.offset_within_address_space;
+
+    return section;
+}
+
 static void mem_add(MemoryListener *listener, MemoryRegionSection *section)
 {
     AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch, listener);
-    MemoryRegionSection now = *section, remain = *section;
+    MemoryRegionSection now = limit(*section), remain = limit(*section);
 
     if ((now.offset_within_address_space & ~TARGET_PAGE_MASK)
         || (now.size < TARGET_PAGE_SIZE)) {



-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] pci: avoid destroying bridge address space windows in a transaction

[Aurelien Jarno]

On Thu, Oct 25, 2012 at 12:37:57PM +0200, Avi Kivity wrote:
> Calling memory_region_destroy() in a transaction is illegal (and aborts),
> as until the transaction is committed, the region remains live.
> 
> Fix by moving destruction until after the transaction commits.  This requires
> having an extra set of regions, so the new and old regions can coexist.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> 
> Please review, and check if this patch fixes things for you.
> 
>  hw/pci_bridge.c    | 50 ++++++++++++++++++++++++++++----------------------
>  hw/pci_internals.h | 24 ++++++++++++++++--------
>  2 files changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
> index 5c6455f..4680501 100644
> --- a/hw/pci_bridge.c
> +++ b/hw/pci_bridge.c
> @@ -151,58 +151,63 @@ static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
>      memory_region_add_subregion_overlap(parent_space, base, alias, 1);
>  }
>  
> -static void pci_bridge_cleanup_alias(MemoryRegion *alias,
> -                                     MemoryRegion *parent_space)
> -{
> -    memory_region_del_subregion(parent_space, alias);
> -    memory_region_destroy(alias);
> -}
> -
> -static void pci_bridge_region_init(PCIBridge *br)
> +static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
>  {
>      PCIBus *parent = br->dev.bus;
> +    PCIBridgeWindows *w = g_new(PCIBridgeWindows, 1);
>      uint16_t cmd = pci_get_word(br->dev.config + PCI_COMMAND);
>  
> -    pci_bridge_init_alias(br, &br->alias_pref_mem,
> +    pci_bridge_init_alias(br, &w->alias_pref_mem,
>                            PCI_BASE_ADDRESS_MEM_PREFETCH,
>                            "pci_bridge_pref_mem",
>                            &br->address_space_mem,
>                            parent->address_space_mem,
>                            cmd & PCI_COMMAND_MEMORY);
> -    pci_bridge_init_alias(br, &br->alias_mem,
> +    pci_bridge_init_alias(br, &w->alias_mem,
>                            PCI_BASE_ADDRESS_SPACE_MEMORY,
>                            "pci_bridge_mem",
>                            &br->address_space_mem,
>                            parent->address_space_mem,
>                            cmd & PCI_COMMAND_MEMORY);
> -    pci_bridge_init_alias(br, &br->alias_io,
> +    pci_bridge_init_alias(br, &w->alias_io,
>                            PCI_BASE_ADDRESS_SPACE_IO,
>                            "pci_bridge_io",
>                            &br->address_space_io,
>                            parent->address_space_io,
>                            cmd & PCI_COMMAND_IO);
>     /* TODO: optinal VGA and VGA palette snooping support. */
> +
> +    return w;
>  }
>  
> -static void pci_bridge_region_cleanup(PCIBridge *br)
> +static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
>  {
>      PCIBus *parent = br->dev.bus;
> -    pci_bridge_cleanup_alias(&br->alias_io,
> -                             parent->address_space_io);
> -    pci_bridge_cleanup_alias(&br->alias_mem,
> -                             parent->address_space_mem);
> -    pci_bridge_cleanup_alias(&br->alias_pref_mem,
> -                             parent->address_space_mem);
> +
> +    memory_region_del_subregion(parent->address_space_io, &w->alias_io);
> +    memory_region_del_subregion(parent->address_space_mem, &w->alias_mem);
> +    memory_region_del_subregion(parent->address_space_mem, &w->alias_pref_mem);
> +}
> +
> +static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
> +{
> +    memory_region_destroy(&w->alias_io);
> +    memory_region_destroy(&w->alias_mem);
> +    memory_region_destroy(&w->alias_pref_mem);
> +    g_free(w);
>  }
>  
>  static void pci_bridge_update_mappings(PCIBridge *br)
>  {
> +    PCIBridgeWindows *w = br->windows;
> +
>      /* Make updates atomic to: handle the case of one VCPU updating the bridge
>       * while another accesses an unaffected region. */
>      memory_region_transaction_begin();
> -    pci_bridge_region_cleanup(br);
> -    pci_bridge_region_init(br);
> +    pci_bridge_region_del(br, br->windows);
> +    br->windows = pci_bridge_region_init(br);
>      memory_region_transaction_commit();
> +    pci_bridge_region_cleanup(br, w);
>  }
>  
>  /* default write_config function for PCI-to-PCI bridge */
> @@ -326,7 +331,7 @@ int pci_bridge_initfn(PCIDevice *dev)
>      memory_region_init(&br->address_space_mem, "pci_bridge_pci", INT64_MAX);
>      sec_bus->address_space_io = &br->address_space_io;
>      memory_region_init(&br->address_space_io, "pci_bridge_io", 65536);
> -    pci_bridge_region_init(br);
> +    br->windows = pci_bridge_region_init(br);
>      QLIST_INIT(&sec_bus->child);
>      QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
>      return 0;
> @@ -338,7 +343,8 @@ void pci_bridge_exitfn(PCIDevice *pci_dev)
>      PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
>      assert(QLIST_EMPTY(&s->sec_bus.child));
>      QLIST_REMOVE(&s->sec_bus, sibling);
> -    pci_bridge_region_cleanup(s);
> +    pci_bridge_region_del(s, s->windows);
> +    pci_bridge_region_cleanup(s, s->windows);
>      memory_region_destroy(&s->address_space_mem);
>      memory_region_destroy(&s->address_space_io);
>      /* qbus_free() is called automatically by qdev_free() */
> diff --git a/hw/pci_internals.h b/hw/pci_internals.h
> index c931b64..21d0ce6 100644
> --- a/hw/pci_internals.h
> +++ b/hw/pci_internals.h
> @@ -40,6 +40,19 @@ struct PCIBus {
>      int *irq_count;
>  };
>  
> +typedef struct PCIBridgeWindows PCIBridgeWindows;
> +
> +/*
> + * Aliases for each of the address space windows that the bridge
> + * can forward. Mapped into the bridge's parent's address space,
> + * as subregions.
> + */
> +struct PCIBridgeWindows {
> +    MemoryRegion alias_pref_mem;
> +    MemoryRegion alias_mem;
> +    MemoryRegion alias_io;
> +};
> +
>  struct PCIBridge {
>      PCIDevice dev;
>  
> @@ -55,14 +68,9 @@ struct PCIBridge {
>       */
>      MemoryRegion address_space_mem;
>      MemoryRegion address_space_io;
> -    /*
> -     * Aliases for each of the address space windows that the bridge
> -     * can forward. Mapped into the bridge's parent's address space,
> -     * as subregions.
> -     */
> -    MemoryRegion alias_pref_mem;
> -    MemoryRegion alias_mem;
> -    MemoryRegion alias_io;
> +
> +    PCIBridgeWindows *windows;
> +
>      pci_map_irq_fn map_irq;
>      const char *bus_name;
>  };

Tested-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [memory] abort with head a8170e5

[Aurelien Jarno]

On Thu, Oct 25, 2012 at 03:47:34PM +0200, Avi Kivity wrote:
> On 10/24/2012 04:00 PM, Aurelien Jarno wrote:
> > 
> > mips is also broken but by commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d:
> > 
> > | [    0.436000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
> > | Segmentation fault (core dumped)
> > 
> 
> How do you reproduce it?

You can use the mips kernel version 2.6.32 from:
  http://people.debian.org/~aurel32/qemu/mips/

Then just run it with the following command:
  qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta -append "console=tty0"

(You can also get the README command line if you don't care about
downloading the disk image).

> Does this patch fix it for you?

Thanks for this patch. Unfortunately it doesn't. In the mean time, I 
have also found that it's possible to workaround the issue by using 
-vga none or -vga std (instead of the default cirrus). I don't know
if it rings a bell for you.

> From: Avi Kivity <avi@redhat.com>
> Date: Thu, 11 Oct 2012 12:40:24 +0200
> Subject: [PATCH] memory: limit sections in the radix tree to the actual
>  address space size
> 
> The radix tree is statically sized to fit TARGET_PHYS_ADDR_SPACE_BITS.
> If a larger memory region is registered, it will overflow.
> 
> Fix by limiting any section in the radix tree to the supported size.
> 
> This problem was not observed earlier since artificial regions (containers
> and aliases) are eliminated by the memory core, leaving only device regions
> which have reasonable sizes.  An IOMMU however cannot be eliminated by the
> memory core, and may have an artificial size.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> 
> diff --git a/exec.c b/exec.c
> index b0ed593..deee8ec 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2280,10 +2280,23 @@ static void register_multipage(AddressSpaceDispatch *d, MemoryRegionSection *sec
>                    section_index);
>  }
>  
> +static MemoryRegionSection limit(MemoryRegionSection section)
> +{
> +    unsigned practical_as_bits = MIN(TARGET_PHYS_ADDR_SPACE_BITS, 62);
> +    hwaddr as_limit;
> +
> +    as_limit = (hwaddr)1 << practical_as_bits;
> +
> +    section.size = MIN(section.offset_within_address_space + section.size, as_limit)
> +                   - section.offset_within_address_space;
> +
> +    return section;
> +}
> +
>  static void mem_add(MemoryListener *listener, MemoryRegionSection *section)
>  {
>      AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch, listener);
> -    MemoryRegionSection now = *section, remain = *section;
> +    MemoryRegionSection now = limit(*section), remain = limit(*section);
>  
>      if ((now.offset_within_address_space & ~TARGET_PAGE_MASK)
>          || (now.size < TARGET_PAGE_SIZE)) {
> 
> 
> 
> -- 
> error compiling committee.c: too many arguments to function
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [memory] abort with head a8170e5

[Avi Kivity]

On 10/25/2012 04:39 PM, Aurelien Jarno wrote:
> On Thu, Oct 25, 2012 at 03:47:34PM +0200, Avi Kivity wrote:
>> On 10/24/2012 04:00 PM, Aurelien Jarno wrote:
>> > 
>> > mips is also broken but by commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d:
>> > 
>> > | [    0.436000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
>> > | Segmentation fault (core dumped)
>> > 
>> 
>> How do you reproduce it?
> 
> You can use the mips kernel version 2.6.32 from:
>   http://people.debian.org/~aurel32/qemu/mips/
> 
> Then just run it with the following command:
>   qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta -append "console=tty0"
> 
> (You can also get the README command line if you don't care about
> downloading the disk image).

Doesn't reproduce here with this command line (upstream + the bridge patch).

[    0.568000] PCI: Enabling device 0000:00:12.0 (0000 -> 0002)
[    0.572000] cirrusfb 0000:00:12.0: Cirrus Logic chipset on PCI bus,
RAM (4096 kB) at 0x10000000

...

[    1.172000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
[    1.188000] scsi0 : ata_piix

(with console=ttyS0)

What's lp - p when the segfault occurs?  What's *index?

| #3  0x00007f4e10f3477f in phys_page_set (leaf=<optimized out>, nb=16,
index=65696, d=0x7f4e124ffb50) at /home/aurel32/qemu/exec.c:458

We're setting 16 pages around address 269090816.  Should be totally
straightforward.

If you make memory_region_transaction_begin()/_commit() no-ops, we can
get a clearer stack trace.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [memory] abort with head a8170e5

[Aurelien Jarno]

On Thu, Oct 25, 2012 at 06:12:06PM +0200, Avi Kivity wrote:
> On 10/25/2012 04:39 PM, Aurelien Jarno wrote:
> > On Thu, Oct 25, 2012 at 03:47:34PM +0200, Avi Kivity wrote:
> >> On 10/24/2012 04:00 PM, Aurelien Jarno wrote:
> >> > 
> >> > mips is also broken but by commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d:
> >> > 
> >> > | [    0.436000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
> >> > | Segmentation fault (core dumped)
> >> > 
> >> 
> >> How do you reproduce it?
> > 
> > You can use the mips kernel version 2.6.32 from:
> >   http://people.debian.org/~aurel32/qemu/mips/
> > 
> > Then just run it with the following command:
> >   qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta -append "console=tty0"
> > 
> > (You can also get the README command line if you don't care about
> > downloading the disk image).
> 
> Doesn't reproduce here with this command line (upstream + the bridge patch).
> 
> [    0.568000] PCI: Enabling device 0000:00:12.0 (0000 -> 0002)
> [    0.572000] cirrusfb 0000:00:12.0: Cirrus Logic chipset on PCI bus,
> RAM (4096 kB) at 0x10000000
> 
> ...
> 
> [    1.172000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
> [    1.188000] scsi0 : ata_piix
> 
> (with console=ttyS0)

Ok, looks like I didn't provide the right command line. I am only able
to reproduce it when using -nographic, and only with -vga cirrus (yes it
starts to be quite strange). In that case it's better to pass 
console=ttyS0, even if you can reproduce it with console=tty0.

In short it seems heavily related to the cirrus VGA card.

> What's lp - p when the segfault occurs?  What's *index?

lp - p = 0xa0
*index = 0x100a0

> | #3  0x00007f4e10f3477f in phys_page_set (leaf=<optimized out>, nb=16,
> index=65696, d=0x7f4e124ffb50) at /home/aurel32/qemu/exec.c:458
> 
> We're setting 16 pages around address 269090816.  Should be totally
> straightforward.
> 
> If you make memory_region_transaction_begin()/_commit() no-ops, we can
> get a clearer stack trace.

I'll try to get that.

Thanks,
Aurelien

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH] pci: avoid destroying bridge address space windows in a transaction

[Michael S. Tsirkin]

On Thu, Oct 25, 2012 at 12:37:57PM +0200, Avi Kivity wrote:
> Calling memory_region_destroy() in a transaction is illegal (and aborts),
> as until the transaction is committed, the region remains live.
> 
> Fix by moving destruction until after the transaction commits.  This requires
> having an extra set of regions, so the new and old regions can coexist.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> 
> Please review, and check if this patch fixes things for you.
> 
>  hw/pci_bridge.c    | 50 ++++++++++++++++++++++++++++----------------------
>  hw/pci_internals.h | 24 ++++++++++++++++--------
>  2 files changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
> index 5c6455f..4680501 100644
> --- a/hw/pci_bridge.c
> +++ b/hw/pci_bridge.c
> @@ -151,58 +151,63 @@ static void pci_bridge_init_alias(PCIBridge *bridge, MemoryRegion *alias,
>      memory_region_add_subregion_overlap(parent_space, base, alias, 1);
>  }
>  
> -static void pci_bridge_cleanup_alias(MemoryRegion *alias,
> -                                     MemoryRegion *parent_space)
> -{
> -    memory_region_del_subregion(parent_space, alias);
> -    memory_region_destroy(alias);
> -}
> -
> -static void pci_bridge_region_init(PCIBridge *br)
> +static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br)
>  {
>      PCIBus *parent = br->dev.bus;
> +    PCIBridgeWindows *w = g_new(PCIBridgeWindows, 1);
>      uint16_t cmd = pci_get_word(br->dev.config + PCI_COMMAND);
>  
> -    pci_bridge_init_alias(br, &br->alias_pref_mem,
> +    pci_bridge_init_alias(br, &w->alias_pref_mem,
>                            PCI_BASE_ADDRESS_MEM_PREFETCH,
>                            "pci_bridge_pref_mem",
>                            &br->address_space_mem,
>                            parent->address_space_mem,
>                            cmd & PCI_COMMAND_MEMORY);
> -    pci_bridge_init_alias(br, &br->alias_mem,
> +    pci_bridge_init_alias(br, &w->alias_mem,
>                            PCI_BASE_ADDRESS_SPACE_MEMORY,
>                            "pci_bridge_mem",
>                            &br->address_space_mem,
>                            parent->address_space_mem,
>                            cmd & PCI_COMMAND_MEMORY);
> -    pci_bridge_init_alias(br, &br->alias_io,
> +    pci_bridge_init_alias(br, &w->alias_io,
>                            PCI_BASE_ADDRESS_SPACE_IO,
>                            "pci_bridge_io",
>                            &br->address_space_io,
>                            parent->address_space_io,
>                            cmd & PCI_COMMAND_IO);
>     /* TODO: optinal VGA and VGA palette snooping support. */
> +
> +    return w;
>  }
>  
> -static void pci_bridge_region_cleanup(PCIBridge *br)
> +static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
>  {
>      PCIBus *parent = br->dev.bus;
> -    pci_bridge_cleanup_alias(&br->alias_io,
> -                             parent->address_space_io);
> -    pci_bridge_cleanup_alias(&br->alias_mem,
> -                             parent->address_space_mem);
> -    pci_bridge_cleanup_alias(&br->alias_pref_mem,
> -                             parent->address_space_mem);
> +
> +    memory_region_del_subregion(parent->address_space_io, &w->alias_io);
> +    memory_region_del_subregion(parent->address_space_mem, &w->alias_mem);
> +    memory_region_del_subregion(parent->address_space_mem, &w->alias_pref_mem);
> +}
> +
> +static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
> +{
> +    memory_region_destroy(&w->alias_io);
> +    memory_region_destroy(&w->alias_mem);
> +    memory_region_destroy(&w->alias_pref_mem);
> +    g_free(w);
>  }
>  
>  static void pci_bridge_update_mappings(PCIBridge *br)
>  {
> +    PCIBridgeWindows *w = br->windows;
> +
>      /* Make updates atomic to: handle the case of one VCPU updating the bridge
>       * while another accesses an unaffected region. */
>      memory_region_transaction_begin();
> -    pci_bridge_region_cleanup(br);
> -    pci_bridge_region_init(br);
> +    pci_bridge_region_del(br, br->windows);
> +    br->windows = pci_bridge_region_init(br);
>      memory_region_transaction_commit();
> +    pci_bridge_region_cleanup(br, w);
>  }
>  
>  /* default write_config function for PCI-to-PCI bridge */
> @@ -326,7 +331,7 @@ int pci_bridge_initfn(PCIDevice *dev)
>      memory_region_init(&br->address_space_mem, "pci_bridge_pci", INT64_MAX);
>      sec_bus->address_space_io = &br->address_space_io;
>      memory_region_init(&br->address_space_io, "pci_bridge_io", 65536);
> -    pci_bridge_region_init(br);
> +    br->windows = pci_bridge_region_init(br);
>      QLIST_INIT(&sec_bus->child);
>      QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
>      return 0;
> @@ -338,7 +343,8 @@ void pci_bridge_exitfn(PCIDevice *pci_dev)
>      PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
>      assert(QLIST_EMPTY(&s->sec_bus.child));
>      QLIST_REMOVE(&s->sec_bus, sibling);
> -    pci_bridge_region_cleanup(s);
> +    pci_bridge_region_del(s, s->windows);
> +    pci_bridge_region_cleanup(s, s->windows);
>      memory_region_destroy(&s->address_space_mem);
>      memory_region_destroy(&s->address_space_io);
>      /* qbus_free() is called automatically by qdev_free() */
> diff --git a/hw/pci_internals.h b/hw/pci_internals.h
> index c931b64..21d0ce6 100644
> --- a/hw/pci_internals.h
> +++ b/hw/pci_internals.h
> @@ -40,6 +40,19 @@ struct PCIBus {
>      int *irq_count;
>  };
>  
> +typedef struct PCIBridgeWindows PCIBridgeWindows;
> +
> +/*
> + * Aliases for each of the address space windows that the bridge
> + * can forward. Mapped into the bridge's parent's address space,
> + * as subregions.
> + */
> +struct PCIBridgeWindows {
> +    MemoryRegion alias_pref_mem;
> +    MemoryRegion alias_mem;
> +    MemoryRegion alias_io;
> +};
> +
>  struct PCIBridge {
>      PCIDevice dev;
>  
> @@ -55,14 +68,9 @@ struct PCIBridge {
>       */
>      MemoryRegion address_space_mem;
>      MemoryRegion address_space_io;
> -    /*
> -     * Aliases for each of the address space windows that the bridge
> -     * can forward. Mapped into the bridge's parent's address space,
> -     * as subregions.
> -     */
> -    MemoryRegion alias_pref_mem;
> -    MemoryRegion alias_mem;
> -    MemoryRegion alias_io;
> +
> +    PCIBridgeWindows *windows;
> +
>      pci_map_irq_fn map_irq;
>      const char *bus_name;
>  };
> -- 
> 1.7.12

Re: [Qemu-devel] [memory] abort with head a8170e5

[Avi Kivity]

On 10/29/2012 09:54 AM, Aurelien Jarno wrote:
> On Thu, Oct 25, 2012 at 06:12:06PM +0200, Avi Kivity wrote:
>> On 10/25/2012 04:39 PM, Aurelien Jarno wrote:
>> > On Thu, Oct 25, 2012 at 03:47:34PM +0200, Avi Kivity wrote:
>> >> On 10/24/2012 04:00 PM, Aurelien Jarno wrote:
>> >> > 
>> >> > mips is also broken but by commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d:
>> >> > 
>> >> > | [    0.436000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
>> >> > | Segmentation fault (core dumped)
>> >> > 
>> >> 
>> >> How do you reproduce it?
>> > 
>> > You can use the mips kernel version 2.6.32 from:
>> >   http://people.debian.org/~aurel32/qemu/mips/
>> > 
>> > Then just run it with the following command:
>> >   qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta -append "console=tty0"
>> > 
>> > (You can also get the README command line if you don't care about
>> > downloading the disk image).
>> 
>> Doesn't reproduce here with this command line (upstream + the bridge patch).
>> 
>> [    0.568000] PCI: Enabling device 0000:00:12.0 (0000 -> 0002)
>> [    0.572000] cirrusfb 0000:00:12.0: Cirrus Logic chipset on PCI bus,
>> RAM (4096 kB) at 0x10000000
>> 
>> ...
>> 
>> [    1.172000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
>> [    1.188000] scsi0 : ata_piix
>> 
>> (with console=ttyS0)
> 
> Ok, looks like I didn't provide the right command line. I am only able
> to reproduce it when using -nographic, and only with -vga cirrus (yes it
> starts to be quite strange). In that case it's better to pass 
> console=ttyS0, even if you can reproduce it with console=tty0.
> 
> In short it seems heavily related to the cirrus VGA card.

I was able to reproduce it, and in fact it's unrelated to VGA, it's deep in the memory core.

Please try this patch:

From: Avi Kivity <avi@redhat.com>
Date: Mon, 29 Oct 2012 17:07:09 +0200
Subject: [PATCH] memory: fix rendering of a region obscured by another

The memory core drops regions that are hidden by another region (for example,
during BAR sizing), but it doesn't do so correctly if the lower address of the
existing range is below the lower address of the new range.

Example (qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta
         -append "console=ttyS0"  -nographic -vga cirrus):

Existing range: 10000000-107fffff
New range:      100a0000-100bffff

Correct behaviour: drop new range
Incorrect behaviour: add new range

Fix by taking this case into account (previously we only considered
equal lower boundaries).

Signed-off-by: Avi Kivity <avi@redhat.com>

diff --git a/memory.c b/memory.c
index 36bb9a5..243cb23 100644
--- a/memory.c
+++ b/memory.c
@@ -539,12 +539,12 @@ static void render_memory_region(FlatView *view,
             offset_in_region += int128_get64(now);
             int128_subfrom(&remain, now);
         }
-        if (int128_eq(base, view->ranges[i].addr.start)) {
-            now = int128_min(remain, view->ranges[i].addr.size);
-            int128_addto(&base, now);
-            offset_in_region += int128_get64(now);
-            int128_subfrom(&remain, now);
-        }
+        now = int128_sub(int128_min(int128_add(base, remain),
+                                    addrrange_end(view->ranges[i].addr)),
+                         base);
+        int128_addto(&base, now);
+        offset_in_region += int128_get64(now);
+        int128_subfrom(&remain, now);
     }
     if (int128_nz(remain)) {
         fr.mr = mr;



-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [memory] abort with head a8170e5

[Aurelien Jarno]

On Mon, Oct 29, 2012 at 05:17:05PM +0200, Avi Kivity wrote:
> On 10/29/2012 09:54 AM, Aurelien Jarno wrote:
> > On Thu, Oct 25, 2012 at 06:12:06PM +0200, Avi Kivity wrote:
> >> On 10/25/2012 04:39 PM, Aurelien Jarno wrote:
> >> > On Thu, Oct 25, 2012 at 03:47:34PM +0200, Avi Kivity wrote:
> >> >> On 10/24/2012 04:00 PM, Aurelien Jarno wrote:
> >> >> > 
> >> >> > mips is also broken but by commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d:
> >> >> > 
> >> >> > | [    0.436000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
> >> >> > | Segmentation fault (core dumped)
> >> >> > 
> >> >> 
> >> >> How do you reproduce it?
> >> > 
> >> > You can use the mips kernel version 2.6.32 from:
> >> >   http://people.debian.org/~aurel32/qemu/mips/
> >> > 
> >> > Then just run it with the following command:
> >> >   qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta -append "console=tty0"
> >> > 
> >> > (You can also get the README command line if you don't care about
> >> > downloading the disk image).
> >> 
> >> Doesn't reproduce here with this command line (upstream + the bridge patch).
> >> 
> >> [    0.568000] PCI: Enabling device 0000:00:12.0 (0000 -> 0002)
> >> [    0.572000] cirrusfb 0000:00:12.0: Cirrus Logic chipset on PCI bus,
> >> RAM (4096 kB) at 0x10000000
> >> 
> >> ...
> >> 
> >> [    1.172000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
> >> [    1.188000] scsi0 : ata_piix
> >> 
> >> (with console=ttyS0)
> > 
> > Ok, looks like I didn't provide the right command line. I am only able
> > to reproduce it when using -nographic, and only with -vga cirrus (yes it
> > starts to be quite strange). In that case it's better to pass 
> > console=ttyS0, even if you can reproduce it with console=tty0.
> > 
> > In short it seems heavily related to the cirrus VGA card.
> 
> I was able to reproduce it, and in fact it's unrelated to VGA, it's deep in the memory core.
> 
> Please try this patch:
> 
> From: Avi Kivity <avi@redhat.com>
> Date: Mon, 29 Oct 2012 17:07:09 +0200
> Subject: [PATCH] memory: fix rendering of a region obscured by another
> 
> The memory core drops regions that are hidden by another region (for example,
> during BAR sizing), but it doesn't do so correctly if the lower address of the
> existing range is below the lower address of the new range.
> 
> Example (qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta
>          -append "console=ttyS0"  -nographic -vga cirrus):
> 
> Existing range: 10000000-107fffff
> New range:      100a0000-100bffff
> 
> Correct behaviour: drop new range
> Incorrect behaviour: add new range
> 
> Fix by taking this case into account (previously we only considered
> equal lower boundaries).
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> 
> diff --git a/memory.c b/memory.c
> index 36bb9a5..243cb23 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -539,12 +539,12 @@ static void render_memory_region(FlatView *view,
>              offset_in_region += int128_get64(now);
>              int128_subfrom(&remain, now);
>          }
> -        if (int128_eq(base, view->ranges[i].addr.start)) {
> -            now = int128_min(remain, view->ranges[i].addr.size);
> -            int128_addto(&base, now);
> -            offset_in_region += int128_get64(now);
> -            int128_subfrom(&remain, now);
> -        }
> +        now = int128_sub(int128_min(int128_add(base, remain),
> +                                    addrrange_end(view->ranges[i].addr)),
> +                         base);
> +        int128_addto(&base, now);
> +        offset_in_region += int128_get64(now);
> +        int128_subfrom(&remain, now);
>      }
>      if (int128_nz(remain)) {
>          fr.mr = mr;
> 

Thanks a lot for the patch, it fixes the problem and I have been able to
boot a MIPS guest up to the login prompt.

Tested-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net