Skip to content

Commit

Permalink
softmmu/ioport.c: make portio_list MemoryRegions children of MemoryRe…
Browse files Browse the repository at this point in the history
…gionPortioList

Currently when portio_list MemoryRegions are freed using portio_list_destroy() the RCU
thread segfaults generating a backtrace similar to that below:

    #0 0x5555599a34b6 in phys_section_destroy ../softmmu/physmem.c:996
    #1 0x5555599a37a3 in phys_sections_free ../softmmu/physmem.c:1011
    #2 0x5555599b24aa in address_space_dispatch_free ../softmmu/physmem.c:2430
    #3 0x55555996a283 in flatview_destroy ../softmmu/memory.c:292
    #4 0x55555a2cb9fb in call_rcu_thread ../util/rcu.c:284
    qemu#5 0x55555a29b71d in qemu_thread_start ../util/qemu-thread-posix.c:541
    qemu#6 0x7ffff4a0cea6 in start_thread nptl/pthread_create.c:477
    qemu#7 0x7ffff492ca2e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfca2e)

The problem here is that portio_list_destroy() unparents the portio_list MemoryRegions
causing them to be freed immediately, however the flatview still has a reference to the
MemoryRegion and so generates a segfault when the RCU thread updates the flatview.

Solve the lifetime issue by making MemoryRegionPortioList a child of the portio_list
owner, and attach the portio_list MemoryRegions to the MemoryRegionPortioList instead of
to the portio_list owner. This ensures that the MemoryRegionPortioList lifecycle is tied
to that of its portio_list MemoryRegions, and allows a finalize() method to be added that
can be called by the (delayed) RCU thread to free the MemoryRegions when the flatview is
next updated.

Since MemoryRegionPortioList is a QOM object with this change the MemoryRegionPortList
is now visible in the output of "info qom-tree". As an example the output of "info
qom-tree" before and after this commit is shown below:

Before:

    /device[4] (i8257)
      /dma-chan[0] (memory-region)
      /dma-cont[0] (memory-region)
      /dma-page[0] (memory-region)
      /dma-page[1] (memory-region)
    /device[5] (i8257)
      /dma-chan[0] (memory-region)
      /dma-cont[0] (memory-region)
      /dma-page[0] (memory-region)
      /dma-page[1] (memory-region)

After:

    /device[4] (i8257)
      /dma-chan[0] (memory-region)
      /dma-cont[0] (memory-region)
      /portiolist[0] (memory-region-portio-list)
        /dma-page[0] (memory-region)
      /portiolist[1] (memory-region-portio-list)
        /dma-page[0] (memory-region)
    /device[5] (i8257)
      /dma-chan[0] (memory-region)
      /dma-cont[0] (memory-region)
      /portiolist[0] (memory-region-portio-list)
        /dma-page[0] (memory-region)
      /portiolist[1] (memory-region-portio-list)
        /dma-page[0] (memory-region)

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
  • Loading branch information
mcayland committed Apr 12, 2023
1 parent ebf3989 commit a9ff1a2
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions softmmu/ioport.c
Expand Up @@ -151,9 +151,7 @@ void portio_list_destroy(PortioList *piolist)

for (i = 0; i < piolist->nr; ++i) {
mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
object_unparent(OBJECT(&mrpio->mr));
g_free(mrpio->ports);
g_free(mrpio);
object_unref(OBJECT(mrpio));
}
g_free(piolist->regions);
}
Expand Down Expand Up @@ -240,13 +238,15 @@ static void portio_list_add_1(PortioList *piolist,
memcpy(mrpio->ports, pio_init, sizeof(MemoryRegionPortio) * count);
memset(mrpio->ports + count, 0, sizeof(MemoryRegionPortio));

object_property_add_child(piolist->owner, "portiolist[*]", OBJECT(mrpio));

/* Adjust the offsets to all be zero-based for the region. */
for (i = 0; i < count; ++i) {
mrpio->ports[i].offset -= off_low;
mrpio->ports[i].base = start + off_low;
}

memory_region_init_io(&mrpio->mr, piolist->owner, &portio_ops, mrpio,
memory_region_init_io(&mrpio->mr, OBJECT(mrpio), &portio_ops, mrpio,
piolist->name, off_high - off_low);
if (piolist->flush_coalesced_mmio) {
memory_region_set_flush_coalesced(&mrpio->mr);
Expand Down Expand Up @@ -305,10 +305,19 @@ void portio_list_del(PortioList *piolist)
}
}

static void memory_region_portio_list_finalize(Object *obj)
{
MemoryRegionPortioList *mrpio = MEMORY_REGION_PORTIO_LIST(obj);

object_unparent(OBJECT(&mrpio->mr));
g_free(mrpio->ports);
}

static const TypeInfo memory_region_portio_list_info = {
.parent = TYPE_OBJECT,
.name = TYPE_MEMORY_REGION_PORTIO_LIST,
.instance_size = sizeof(MemoryRegionPortioList),
.instance_finalize = memory_region_portio_list_finalize,
};

static void ioport_register_types(void)
Expand Down

0 comments on commit a9ff1a2

Please sign in to comment.