Commit f8ed85ac authored by Markus Armbruster's avatar Markus Armbruster
Browse files

Fix bad error handling after memory_region_init_ram() 



Symptom:

    $ qemu-system-x86_64 -m 10000000
    Unexpected error in ram_block_add() at /work/armbru/qemu/exec.c:1456:
    upstream-qemu: cannot set up guest memory 'pc.ram': Cannot allocate memory
    Aborted (core dumped)

Root cause: commit ef701d7b screwed up handling of out-of-memory
conditions.  Before the commit, we report the error and exit(1), in
one place, ram_block_add().  The commit lifts the error handling up
the call chain some, to three places.  Fine.  Except it uses
&error_abort in these places, changing the behavior from exit(1) to
abort(), and thus undoing the work of commit 39228250 "exec: Don't
abort when we can't allocate guest memory".

The three places are:

* memory_region_init_ram()

  Commit 49946538 (right after commit ef701d7b) lifted the error
  handling further, through memory_region_init_ram(), multiplying the
  incorrect use of &error_abort.  Later on, imitation of existing
  (bad) code may have created more.

* memory_region_init_ram_ptr()

  The &error_abort is still there.

* memory_region_init_rom_device()

  Doesn't need fixing, because commit 33e0eb52 (soon after commit
  ef701d7b) lifted the error handling further, and in the process
  changed it from &error_abort to passing it up the call chain.
  Correct, because the callers are realize() methods.

Fix the error handling after memory_region_init_ram() with a
Coccinelle semantic patch:

    @r@
    expression mr, owner, name, size, err;
    position p;
    @@
            memory_region_init_ram(mr, owner, name, size,
    (
    -                              &error_abort
    +                              &error_fatal
    |
                                   err@p
    )
                                  );
    @script:python@
        p << r.p;
    @@
    print "%s:%s:%s" % (p[0].file, p[0].line, p[0].column)

When the last argument is &error_abort, it gets replaced by
&error_fatal.  This is the fix.

If the last argument is anything else, its position is reported.  This
lets us check the fix is complete.  Four positions get reported:

* ram_backend_memory_alloc()

  Error is passed up the call chain, ultimately through
  user_creatable_complete().  As far as I can tell, it's callers all
  handle the error sanely.

* fsl_imx25_realize(), fsl_imx31_realize(), dp8393x_realize()

  DeviceClass.realize() methods, errors handled sanely further up the
  call chain.

We're good.  Test case again behaves:

    $ qemu-system-x86_64 -m 10000000
    qemu-system-x86_64: cannot set up guest memory 'pc.ram': Cannot allocate memory
    [Exit 1 ]

The next commits will repair the rest of commit ef701d7b's damage.

Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
Message-Id: <1441983105-26376-3-git-send-email-armbru@redhat.com>
Reviewed-by: default avatarPeter Crosthwaite <crosthwaite.peter@gmail.com>
parent a29a37b9

hw/arm/armv7m.c

+1 −1
Original line number Diff line number Diff line
@@ -229,7 +229,7 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, 
    /* Hack to map an additional page of ram at the top of the address

       space.  This stops qemu complaining about executing code outside RAM

       when returning from an exception.  */

    memory_region_init_ram(hack, NULL, "armv7m.hack", 0x1000, &error_abort);

    memory_region_init_ram(hack, NULL, "armv7m.hack", 0x1000, &error_fatal);

    vmstate_register_ram_global(hack);

    memory_region_add_subregion(system_memory, 0xfffff000, hack);

hw/arm/exynos4210.c

+4 −4
Original line number Diff line number Diff line
@@ -259,7 +259,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem, 


    /* Internal ROM */

    memory_region_init_ram(&s->irom_mem, NULL, "exynos4210.irom",

                           EXYNOS4210_IROM_SIZE, &error_abort);

                           EXYNOS4210_IROM_SIZE, &error_fatal);

    vmstate_register_ram_global(&s->irom_mem);

    memory_region_set_readonly(&s->irom_mem, true);

    memory_region_add_subregion(system_mem, EXYNOS4210_IROM_BASE_ADDR,
@@ -275,7 +275,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem, 


    /* Internal RAM */

    memory_region_init_ram(&s->iram_mem, NULL, "exynos4210.iram",

                           EXYNOS4210_IRAM_SIZE, &error_abort);

                           EXYNOS4210_IRAM_SIZE, &error_fatal);

    vmstate_register_ram_global(&s->iram_mem);

    memory_region_add_subregion(system_mem, EXYNOS4210_IRAM_BASE_ADDR,

                                &s->iram_mem);
@@ -284,14 +284,14 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem, 
    mem_size = ram_size;

    if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {

        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",

                mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_abort);

                mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_fatal);

        vmstate_register_ram_global(&s->dram1_mem);

        memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,

                &s->dram1_mem);

        mem_size = EXYNOS4210_DRAM_MAX_SIZE;

    }

    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,

                           &error_abort);

                           &error_fatal);

    vmstate_register_ram_global(&s->dram0_mem);

    memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,

            &s->dram0_mem);

hw/arm/highbank.c

+1 −1
Original line number Diff line number Diff line
@@ -281,7 +281,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id) 


    sysram = g_new(MemoryRegion, 1);

    memory_region_init_ram(sysram, NULL, "highbank.sysram", 0x8000,

                           &error_abort);

                           &error_fatal);

    memory_region_add_subregion(sysmem, 0xfff88000, sysram);

    if (bios_name != NULL) {

        sysboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);

hw/arm/integratorcp.c

+1 −1
Original line number Diff line number Diff line
@@ -266,7 +266,7 @@ static int integratorcm_init(SysBusDevice *dev) 
    s->cm_refcnt_offset = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24,

                                   1000);

    memory_region_init_ram(&s->flash, OBJECT(s), "integrator.flash", 0x100000,

                           &error_abort);

                           &error_fatal);

    vmstate_register_ram_global(&s->flash);



    memory_region_init_io(&s->iomem, OBJECT(s), &integratorcm_ops, s,

hw/arm/mainstone.c

+1 −1
Original line number Diff line number Diff line
@@ -124,7 +124,7 @@ static void mainstone_common_init(MemoryRegion *address_space_mem, 
    /* Setup CPU & memory */

    mpu = pxa270_init(address_space_mem, mainstone_binfo.ram_size, cpu_model);

    memory_region_init_ram(rom, NULL, "mainstone.rom", MAINSTONE_ROM,

                           &error_abort);

                           &error_fatal);

    vmstate_register_ram_global(rom);

    memory_region_set_readonly(rom, true);

    memory_region_add_subregion(address_space_mem, 0, rom);