SYS-OSS/FRET-qemu
4
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

hw/arm/aspeed: Remove extraneous MemoryRegion object owner

Browse Source 
I'm confused by this code, 'bmc' is created as:

  bmc = g_new0(AspeedBoardState, 1);

Then we use it as QOM owner for different MemoryRegion objects.
But looking at memory_region_init_ram (similarly for ROM):

  void memory_region_init_ram(MemoryRegion *mr,
                              struct Object *owner,
                              const char *name,
                              uint64_t size,
                              Error **errp)
  {
      DeviceState *owner_dev;
      Error *err = NULL;

      memory_region_init_ram_nomigrate(mr, owner, name, size, &err);
      if (err) {
          error_propagate(errp, err);
          return;
      }
      /* This will assert if owner is neither NULL nor a DeviceState.
       * We only want the owner here for the purposes of defining a
       * unique name for migration. TODO: Ideally we should implement
       * a naming scheme for Objects which are not DeviceStates, in
       * which case we can relax this restriction.
       */
      owner_dev = DEVICE(owner);
      vmstate_register_ram(mr, owner_dev);
  }

The expected assertion is not triggered ('bmc' is not NULL neither
a DeviceState).

'bmc' structure is defined as:

  struct AspeedBoardState {
      AspeedSoCState soc;
      MemoryRegion ram_container;
      MemoryRegion max_ram;
  };

What happens is when using 'OBJECT(bmc)', the QOM macros cast the
memory pointed by bmc, which first member is 'soc', which is
initialized ...:

  object_initialize_child(OBJECT(machine), "soc",
                          &bmc->soc, amc->soc_name);

The 'soc' object is indeed a DeviceState, so the assertion passes.

Since this is fragile and only happens to work by luck, remove the
dangerous OBJECT(bmc) owner argument.

Note, this probably breaks migration for this machine.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20200623072132.2868-2-f4bug@amsat.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Philippe Mathieu-Daudé 2020-06-23 09:21:30 +02:00 committed by Peter Maydell
parent 10f7ffabf9
commit f489960d36
1 changed files with 3 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
hw/arm/aspeed.c
View File

@ -329,12 +329,12 @@ static void aspeed_machine_init(MachineState *machine)
* needed by the flash modules of the Aspeed machines. * needed by the flash modules of the Aspeed machines.
*/ */
if (ASPEED_MACHINE(machine)->mmio_exec) { if (ASPEED_MACHINE(machine)->mmio_exec) {
memory_region_init_alias(boot_rom, OBJECT(bmc), "aspeed.boot_rom", memory_region_init_alias(boot_rom, NULL, "aspeed.boot_rom",
&fl->mmio, 0, fl->size); &fl->mmio, 0, fl->size);
memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR, memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
boot_rom); boot_rom);
} else { } else {
memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom", memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom",
fl->size, &error_abort); fl->size, &error_abort);
memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR, memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
boot_rom); boot_rom);
@ -345,7 +345,7 @@ static void aspeed_machine_init(MachineState *machine)
if (machine->kernel_filename && sc->num_cpus > 1) { if (machine->kernel_filename && sc->num_cpus > 1) {
/* With no u-boot we must set up a boot stub for the secondary CPU */ /* With no u-boot we must set up a boot stub for the secondary CPU */
MemoryRegion *smpboot = g_new(MemoryRegion, 1); MemoryRegion *smpboot = g_new(MemoryRegion, 1);
memory_region_init_ram(smpboot, OBJECT(bmc), "aspeed.smpboot", memory_region_init_ram(smpboot, NULL, "aspeed.smpboot",
0x80, &error_abort); 0x80, &error_abort);
memory_region_add_subregion(get_system_memory(), memory_region_add_subregion(get_system_memory(),
AST_SMP_MAILBOX_BASE, smpboot); AST_SMP_MAILBOX_BASE, smpboot);