All of lore.kernel.org
 help / color / mirror / Atom feed
* Trying to understand QOM object creation and property linking
@ 2022-01-05 18:03 Alex Bennée
  2022-01-05 19:03 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2022-01-05 18:03 UTC (permalink / raw)
  To: Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost
  Cc: Peter Maydell, qemu-devel

Hi,

I'm having a hell of a time trying to create a new SoC+Board model from
scratch. The problem comes down to trying to expose some properties to
the underlying CPU from my board model. So I have:

  static const TypeInfo pipico_machine_types[] = {
      {
          .name           = TYPE_PIPICO_MACHINE,
          .parent         = TYPE_MACHINE,
          .instance_size  = sizeof(PiPicoMachineState),
          .class_size     = sizeof(PiPicoMachineClass),
          .class_init     = pipico_machine_class_init,
      }
  };

and the class init sets:

    MachineClass *mc = MACHINE_CLASS(oc);
    ...
    mc->desc = g_strdup_printf("Raspberry Pi Pico");
    mc->init = pipico_machine_init;
    ...

and finally when I init the machine I do the following:

static void pipico_machine_init(MachineState *machine)
{
    PiPicoMachineState *s = PIPICO_MACHINE(machine);
    ...
    MemoryRegion *system_memory = get_system_memory();

    ...
    
    /* initialize external Flash device */
    memory_region_init_rom(&s->flash, NULL,
                           "pico.flash0", 256 * KiB, &error_fatal);
    memory_region_add_subregion(system_memory, 0, &s->flash);

    /* Setup the SOC */
    object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_RP2040);

    /* link properties from machine the SoC needs */
    object_property_set_link(OBJECT(&s->soc), "memory",
                             OBJECT(system_memory), &error_fatal);

    sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);


The initialisation of the SoC is simple because I can't do much until
things are realised:

static void rp2040_init(Object *obj)
{
    RP2040State *s = RP2040(obj);
    int n;

    fprintf(stderr, "%s: %p\n", __func__, obj);

    for (n = 0; n < RP2040_NCPUS; n++) {
        object_initialize_child(obj, "cpu[*]", &s->armv7m[n], TYPE_ARMV7M);
        qdev_prop_set_string(DEVICE(&s->armv7m[n]), "cpu-type",
                             ARM_CPU_TYPE_NAME("cortex-m0"));
    }
}


However when I get to realize the SoC itself:

static void rp2040_realize(DeviceState *dev, Error **errp)
{
    RP2040State *s = RP2040(dev);
    Object *obj = OBJECT(dev);
    int n;

    if (!s->board_memory) {
        error_setg(errp, "%s: memory property was not set", __func__);
        return;
    }

    /* initialize internal 16 KB internal ROM */
    memory_region_init_rom(&s->rom, obj, "rp2040.rom0", 16 * KiB, errp);
    memory_region_add_subregion(s->board_memory, 0, &s->rom);

    /* SRAM (Main 256k bank + two 4k banks)*/
    memory_region_init_ram(&s->sram03, obj, "rp2040.sram03", 256 * KiB, errp);
    memory_region_add_subregion(s->board_memory, RP2040_SRAM_BASE, &s->sram03);

    memory_region_init_ram(&s->sram4, obj, "rp2040.sram4", 4 * KiB, errp);
    memory_region_add_subregion(s->board_memory, RP2040_SRAM4_BASE, &s->sram4);

    memory_region_init_ram(&s->sram5, obj, "rp2040.sram5", 4 * KiB, errp);
    memory_region_add_subregion(s->board_memory, RP2040_SRAM5_BASE, &s->sram5);

    ...

    for (n = 0; n < RP2040_NCPUS; n++) {
        /* DeviceState *cpudev = DEVICE(&s->armv7m[i]); */
        Object *cpuobj = OBJECT(&s->armv7m[n]);

        object_property_set_link(cpuobj, "memory",
                                 OBJECT(&s->board_memory), errp);

And this passing of the link down to the CPU I segfault:

  rp2040_init: 0x555556d08710

  Thread 1 "qemu-system-arm" received signal SIGSEGV, Segmentation fault.
  object_get_canonical_path_component (obj=0x555556d0ea28) at ../../qom/object.c:1999
  1999        g_hash_table_iter_init(&iter, obj->parent->properties);
  (gdb) bt
  #0  object_get_canonical_path_component (obj=0x555556d0ea28) at ../../qom/object.c:1999
  #1  0x0000555555fb27ea in object_get_canonical_path (obj=0x555556d0ea28) at ../../qom/object.c:2025
  #2  0x0000555555fb1250 in object_property_set_link (obj=0x555556d087a0, name=0x5555563190a2 "memory", value=0x555556d0ea28, errp=0x7fffffffe0f0) at ../../qom/object.c:1445
  #3  0x0000555555cf3c23 in rp2040_realize (dev=0x555556d08710, errp=0x7fffffffe0f0) at ../../hw/arm/rp2040.c:85
  #4  0x0000555555fa9323 in device_set_realized (obj=0x555556d08710, value=true, errp=0x7fffffffe200) at ../../hw/core/qdev.c:532
  #5  0x0000555555fb300d in property_set_bool (obj=0x555556d08710, v=0x555556dced10, name=0x5555563822b9 "realized", opaque=0x555556a3a6d0, errp=0x7fffffffe200) at ../../qom/object.c:2268
  #6  0x0000555555fb1054 in object_property_set (obj=0x555556d08710, name=0x5555563822b9 "realized", v=0x555556dced10, errp=0x7fffffffe200) at ../../qom/object.c:1403
  #7  0x0000555555fb53ff in object_property_set_qobject (obj=0x555556d08710, name=0x5555563822b9 "realized", value=0x555556e79bc0, errp=0x555556918de0 <error_fatal>) at ../../qom/qom-qobject.c:28
  #8  0x0000555555fb13b9 in object_property_set_bool (obj=0x555556d08710, name=0x5555563822b9 "realized", value=true, errp=0x555556918de0 <error_fatal>) at ../../qom/object.c:1472
  #9  0x0000555555fa8beb in qdev_realize (dev=0x555556d08710, bus=0x555556d0f240, errp=0x555556918de0 <error_fatal>) at ../../hw/core/qdev.c:334
  #10 0x00005555559f0e28 in sysbus_realize (dev=0x555556d08710, errp=0x555556918de0 <error_fatal>) at ../../hw/core/sysbus.c:256
  #11 0x0000555555cf3f0e in pipico_machine_init (machine=0x555556d08600) at ../../hw/arm/raspi_pico.c:74
  #12 0x00005555559ed71b in machine_run_board_init (machine=0x555556d08600) at ../../hw/core/machine.c:1184
  #13 0x0000555555e67f2c in qemu_init_board () at ../../softmmu/vl.c:2655
  #14 0x0000555555e6814a in qmp_x_exit_preconfig (errp=0x555556918de0 <error_fatal>) at ../../softmmu/vl.c:2743
  #15 0x0000555555e6a811 in qemu_init (argc=3, argv=0x7fffffffe6b8, envp=0x7fffffffe6d8) at ../../softmmu/vl.c:3778
  #16 0x0000555555884ebd in main (argc=3, argv=0x7fffffffe6b8, envp=0x7fffffffe6d8) at ../../softmmu/main.c:49

So have I discovered a bug in QOM handling or misunderstood the way
properties are meant to be shared from the main machine to the
underlying CPU?

Follow-up questions, does only creating the main memory aliases as part
of the SoC make sense? My rational is most of the memory is part of the
SoC not the board. I assume later RP2040 based boards may have different
flash configs or even external memory.

The current (messy) state of my tree can be seen at:

  https://gitlab.com/stsquad/qemu/-/commits/arm/picopi-rfc

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Trying to understand QOM object creation and property linking
  2022-01-05 18:03 Trying to understand QOM object creation and property linking Alex Bennée
@ 2022-01-05 19:03 ` Philippe Mathieu-Daudé
  2022-01-05 21:02   ` Alex Bennée
  0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-01-05 19:03 UTC (permalink / raw)
  To: Alex Bennée, Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost
  Cc: Peter Maydell, qemu-devel

Hi Alex,

On 5/1/22 19:03, Alex Bennée wrote:
> Hi,
> 
> I'm having a hell of a time trying to create a new SoC+Board model from
> scratch. The problem comes down to trying to expose some properties to
> the underlying CPU from my board model. So I have:
> 
>    static const TypeInfo pipico_machine_types[] = {
>        {
>            .name           = TYPE_PIPICO_MACHINE,
>            .parent         = TYPE_MACHINE,
>            .instance_size  = sizeof(PiPicoMachineState),
>            .class_size     = sizeof(PiPicoMachineClass),
>            .class_init     = pipico_machine_class_init,
>        }
>    };
> 
> and the class init sets:
> 
>      MachineClass *mc = MACHINE_CLASS(oc);
>      ...
>      mc->desc = g_strdup_printf("Raspberry Pi Pico");
>      mc->init = pipico_machine_init;
>      ...
> 
> and finally when I init the machine I do the following:
> 
> static void pipico_machine_init(MachineState *machine)
> {
>      PiPicoMachineState *s = PIPICO_MACHINE(machine);
>      ...
>      MemoryRegion *system_memory = get_system_memory();
> 
>      ...
>      
>      /* initialize external Flash device */
>      memory_region_init_rom(&s->flash, NULL,
>                             "pico.flash0", 256 * KiB, &error_fatal);
>      memory_region_add_subregion(system_memory, 0, &s->flash);
> 
>      /* Setup the SOC */
>      object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_RP2040);
> 
>      /* link properties from machine the SoC needs */
>      object_property_set_link(OBJECT(&s->soc), "memory",
>                               OBJECT(system_memory), &error_fatal);
> 
>      sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);
> 
> 
> The initialisation of the SoC is simple because I can't do much until
> things are realised:
> 
> static void rp2040_init(Object *obj)
> {
>      RP2040State *s = RP2040(obj);
>      int n;
> 
>      fprintf(stderr, "%s: %p\n", __func__, obj);
> 
>      for (n = 0; n < RP2040_NCPUS; n++) {
>          object_initialize_child(obj, "cpu[*]", &s->armv7m[n], TYPE_ARMV7M);
>          qdev_prop_set_string(DEVICE(&s->armv7m[n]), "cpu-type",
>                               ARM_CPU_TYPE_NAME("cortex-m0"));

Here for each core you need to initialize a MemoryRegion container, ...

>      }
> }
> 
> 
> However when I get to realize the SoC itself:
> 
> static void rp2040_realize(DeviceState *dev, Error **errp)
> {
>      RP2040State *s = RP2040(dev);
>      Object *obj = OBJECT(dev);
>      int n;
> 
>      if (!s->board_memory) {
>          error_setg(errp, "%s: memory property was not set", __func__);
>          return;
>      }
> 
>      /* initialize internal 16 KB internal ROM */
>      memory_region_init_rom(&s->rom, obj, "rp2040.rom0", 16 * KiB, errp);
>      memory_region_add_subregion(s->board_memory, 0, &s->rom);
> 
>      /* SRAM (Main 256k bank + two 4k banks)*/
>      memory_region_init_ram(&s->sram03, obj, "rp2040.sram03", 256 * KiB, errp);
>      memory_region_add_subregion(s->board_memory, RP2040_SRAM_BASE, &s->sram03);
> 
>      memory_region_init_ram(&s->sram4, obj, "rp2040.sram4", 4 * KiB, errp);
>      memory_region_add_subregion(s->board_memory, RP2040_SRAM4_BASE, &s->sram4);
> 
>      memory_region_init_ram(&s->sram5, obj, "rp2040.sram5", 4 * KiB, errp);
>      memory_region_add_subregion(s->board_memory, RP2040_SRAM5_BASE, &s->sram5);
> 
>      ...
> 
>      for (n = 0; n < RP2040_NCPUS; n++) {
>          /* DeviceState *cpudev = DEVICE(&s->armv7m[i]); */
>          Object *cpuobj = OBJECT(&s->armv7m[n]);

then you add the board_memory in the per-CPU container as subregion, ...

>          object_property_set_link(cpuobj, "memory",
>                                   OBJECT(&s->board_memory), errp);

and finally each CPU links its container as its memory bus.

> And this passing of the link down to the CPU I segfault:
> 
>    rp2040_init: 0x555556d08710
> 
>    Thread 1 "qemu-system-arm" received signal SIGSEGV, Segmentation fault.
>    object_get_canonical_path_component (obj=0x555556d0ea28) at ../../qom/object.c:1999
>    1999        g_hash_table_iter_init(&iter, obj->parent->properties);
>    (gdb) bt
>    #0  object_get_canonical_path_component (obj=0x555556d0ea28) at ../../qom/object.c:1999
>    #1  0x0000555555fb27ea in object_get_canonical_path (obj=0x555556d0ea28) at ../../qom/object.c:2025
>    #2  0x0000555555fb1250 in object_property_set_link (obj=0x555556d087a0, name=0x5555563190a2 "memory", value=0x555556d0ea28, errp=0x7fffffffe0f0) at ../../qom/object.c:1445
>    #3  0x0000555555cf3c23 in rp2040_realize (dev=0x555556d08710, errp=0x7fffffffe0f0) at ../../hw/arm/rp2040.c:85
>    #4  0x0000555555fa9323 in device_set_realized (obj=0x555556d08710, value=true, errp=0x7fffffffe200) at ../../hw/core/qdev.c:532
>    #5  0x0000555555fb300d in property_set_bool (obj=0x555556d08710, v=0x555556dced10, name=0x5555563822b9 "realized", opaque=0x555556a3a6d0, errp=0x7fffffffe200) at ../../qom/object.c:2268
>    #6  0x0000555555fb1054 in object_property_set (obj=0x555556d08710, name=0x5555563822b9 "realized", v=0x555556dced10, errp=0x7fffffffe200) at ../../qom/object.c:1403
>    #7  0x0000555555fb53ff in object_property_set_qobject (obj=0x555556d08710, name=0x5555563822b9 "realized", value=0x555556e79bc0, errp=0x555556918de0 <error_fatal>) at ../../qom/qom-qobject.c:28
>    #8  0x0000555555fb13b9 in object_property_set_bool (obj=0x555556d08710, name=0x5555563822b9 "realized", value=true, errp=0x555556918de0 <error_fatal>) at ../../qom/object.c:1472
>    #9  0x0000555555fa8beb in qdev_realize (dev=0x555556d08710, bus=0x555556d0f240, errp=0x555556918de0 <error_fatal>) at ../../hw/core/qdev.c:334
>    #10 0x00005555559f0e28 in sysbus_realize (dev=0x555556d08710, errp=0x555556918de0 <error_fatal>) at ../../hw/core/sysbus.c:256
>    #11 0x0000555555cf3f0e in pipico_machine_init (machine=0x555556d08600) at ../../hw/arm/raspi_pico.c:74
>    #12 0x00005555559ed71b in machine_run_board_init (machine=0x555556d08600) at ../../hw/core/machine.c:1184
>    #13 0x0000555555e67f2c in qemu_init_board () at ../../softmmu/vl.c:2655
>    #14 0x0000555555e6814a in qmp_x_exit_preconfig (errp=0x555556918de0 <error_fatal>) at ../../softmmu/vl.c:2743
>    #15 0x0000555555e6a811 in qemu_init (argc=3, argv=0x7fffffffe6b8, envp=0x7fffffffe6d8) at ../../softmmu/vl.c:3778
>    #16 0x0000555555884ebd in main (argc=3, argv=0x7fffffffe6b8, envp=0x7fffffffe6d8) at ../../softmmu/main.c:49
> 
> So have I discovered a bug in QOM handling or misunderstood the way
> properties are meant to be shared from the main machine to the
> underlying CPU?
> 
> Follow-up questions, does only creating the main memory aliases as part
> of the SoC make sense? My rational is most of the memory is part of the
> SoC not the board. I assume later RP2040 based boards may have different
> flash configs or even external memory.
> 
> The current (messy) state of my tree can be seen at:
> 
>    https://gitlab.com/stsquad/qemu/-/commits/arm/picopi-rfc
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Trying to understand QOM object creation and property linking
  2022-01-05 19:03 ` Philippe Mathieu-Daudé
@ 2022-01-05 21:02   ` Alex Bennée
  2022-01-06 11:16     ` Peter Maydell
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2022-01-05 21:02 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Eduardo Habkost, Paolo Bonzini, Daniel P. Berrange, qemu-devel,
	Peter Maydell


Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Hi Alex,
>
> On 5/1/22 19:03, Alex Bennée wrote:
>> Hi,
>> I'm having a hell of a time trying to create a new SoC+Board model
>> from
>> scratch. The problem comes down to trying to expose some properties to
>> the underlying CPU from my board model. So I have:
<snip>
>> static void pipico_machine_init(MachineState *machine)
>> {
>>      PiPicoMachineState *s = PIPICO_MACHINE(machine);
>>      ...
>>      MemoryRegion *system_memory = get_system_memory();

Hmm this memory is initialised by memory_region_init() so...

>>      ...
>>           /* initialize external Flash device */
>>      memory_region_init_rom(&s->flash, NULL,
>>                             "pico.flash0", 256 * KiB, &error_fatal);
>>      memory_region_add_subregion(system_memory, 0, &s->flash);
>>      /* Setup the SOC */
>>      object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_RP2040);
>>      /* link properties from machine the SoC needs */
>>      object_property_set_link(OBJECT(&s->soc), "memory",
>>                               OBJECT(system_memory), &error_fatal);
>>      sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);
>> The initialisation of the SoC is simple because I can't do much
>> until
>> things are realised:
>> static void rp2040_init(Object *obj)
>> {
>>      RP2040State *s = RP2040(obj);
>>      int n;
>>      fprintf(stderr, "%s: %p\n", __func__, obj);
>>      for (n = 0; n < RP2040_NCPUS; n++) {
>>          object_initialize_child(obj, "cpu[*]", &s->armv7m[n], TYPE_ARMV7M);
>>          qdev_prop_set_string(DEVICE(&s->armv7m[n]), "cpu-type",
>>                               ARM_CPU_TYPE_NAME("cortex-m0"));
>
> Here for each core you need to initialize a MemoryRegion container, ...
>
>>      }
>> }
>> However when I get to realize the SoC itself:
>> static void rp2040_realize(DeviceState *dev, Error **errp)
>> {
>>      RP2040State *s = RP2040(dev);
>>      Object *obj = OBJECT(dev);
>>      int n;
>>      if (!s->board_memory) {
>>          error_setg(errp, "%s: memory property was not set", __func__);
>>          return;
>>      }
>>      /* initialize internal 16 KB internal ROM */
>>      memory_region_init_rom(&s->rom, obj, "rp2040.rom0", 16 * KiB, errp);
>>      memory_region_add_subregion(s->board_memory, 0, &s->rom);
>>      /* SRAM (Main 256k bank + two 4k banks)*/
>>      memory_region_init_ram(&s->sram03, obj, "rp2040.sram03", 256 * KiB, errp);
>>      memory_region_add_subregion(s->board_memory, RP2040_SRAM_BASE, &s->sram03);
>>      memory_region_init_ram(&s->sram4, obj, "rp2040.sram4", 4 * KiB,
>> errp);
>>      memory_region_add_subregion(s->board_memory, RP2040_SRAM4_BASE, &s->sram4);
>>      memory_region_init_ram(&s->sram5, obj, "rp2040.sram5", 4 * KiB,
>> errp);
>>      memory_region_add_subregion(s->board_memory, RP2040_SRAM5_BASE, &s->sram5);
>>      ...
>>      for (n = 0; n < RP2040_NCPUS; n++) {
>>          /* DeviceState *cpudev = DEVICE(&s->armv7m[i]); */
>>          Object *cpuobj = OBJECT(&s->armv7m[n]);
>
> then you add the board_memory in the per-CPU container as subregion,
> ...

Can't be added as a subregion to the container...

  qemu-system-arm: ../../softmmu/memory.c:2538: memory_region_add_subregion_common: Assertion `!subregion->container' failed.

>
>>          object_property_set_link(cpuobj, "memory",
>>                                   OBJECT(&s->board_memory), errp);
>
> and finally each CPU links its container as its memory bus.

So something like:

        object_property_set_link(cpuobj, "memory", OBJECT(s->cpu_mem[n]), errp);

so the CPU sees the container with whatever particular set of memory
regions you want to make visible to that CPU?

<snip>

>> Follow-up questions, does only creating the main memory aliases as
>> part
>> of the SoC make sense? My rational is most of the memory is part of the
>> SoC not the board. I assume later RP2040 based boards may have different
>> flash configs or even external memory.
>> The current (messy) state of my tree can be seen at:
>>    https://gitlab.com/stsquad/qemu/-/commits/arm/picopi-rfc
>> 


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Trying to understand QOM object creation and property linking
  2022-01-05 21:02   ` Alex Bennée
@ 2022-01-06 11:16     ` Peter Maydell
  2022-01-06 14:20       ` Alex Bennée
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2022-01-06 11:16 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Eduardo Habkost, Paolo Bonzini, Daniel P. Berrange,
	Philippe Mathieu-Daudé,
	qemu-devel

On Wed, 5 Jan 2022 at 21:05, Alex Bennée <alex.bennee@linaro.org> wrote:
> Can't be added as a subregion to the container...
>
>   qemu-system-arm: ../../softmmu/memory.c:2538: memory_region_add_subregion_common: Assertion `!subregion->container' failed.

This assert means you tried to add the same MemoryRegion
as a subregion of more than one parent MR.

You can either:
 * pass all the CPUs the same container as their "memory" link,
   if they all see the same view of the world
 * if they have different views of the world, you need to
   create a container for each CPU to be the "memory" link,
   and to populate that container you need to create N-1 alias MRs
   of the board_memory MR (CPU 0's container can use the original
   board_memory MR; CPU 1, ... use the aliases).

Example of option 1: virt board
Example of option 2: hw/arm/armsse.c (look at what it does with
the s->cpu_container[] and s->container_alias[] arrays)

-- PMM


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Trying to understand QOM object creation and property linking
  2022-01-06 11:16     ` Peter Maydell
@ 2022-01-06 14:20       ` Alex Bennée
  2022-01-06 15:04         ` Peter Maydell
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2022-01-06 14:20 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Eduardo Habkost, Paolo Bonzini, Daniel P. Berrange,
	Philippe Mathieu-Daudé,
	qemu-devel


Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 5 Jan 2022 at 21:05, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Can't be added as a subregion to the container...
>>
>>   qemu-system-arm: ../../softmmu/memory.c:2538: memory_region_add_subregion_common: Assertion `!subregion->container' failed.
>
> This assert means you tried to add the same MemoryRegion
> as a subregion of more than one parent MR.

Right - that is probably something we should make (more?) explicitly
clear in the Memory API docs.

> You can either:
>  * pass all the CPUs the same container as their "memory" link,
>    if they all see the same view of the world

This should be the case - I don't think the different cores have any
particular different view of the world. The use of the two 4kb banks I
think is purely convention.

However trying for a single container shared between both cores fails
because armv7m_realize adds it's board_memory to another container:

    memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);

So I guess I just have to repeat the creation of the aliases for each
core. This seems needlessly messy...

>  * if they have different views of the world, you need to
>    create a container for each CPU to be the "memory" link,
>    and to populate that container you need to create N-1 alias MRs
>    of the board_memory MR (CPU 0's container can use the original
>    board_memory MR; CPU 1, ... use the aliases).
>
> Example of option 1: virt board
> Example of option 2: hw/arm/armsse.c (look at what it does with
> the s->cpu_container[] and s->container_alias[] arrays)
>
> -- PMM


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Trying to understand QOM object creation and property linking
  2022-01-06 14:20       ` Alex Bennée
@ 2022-01-06 15:04         ` Peter Maydell
  2022-01-06 15:44           ` Alex Bennée
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2022-01-06 15:04 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Eduardo Habkost, Paolo Bonzini, Daniel P. Berrange,
	Philippe Mathieu-Daudé,
	qemu-devel

On Thu, 6 Jan 2022 at 14:26, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Wed, 5 Jan 2022 at 21:05, Alex Bennée <alex.bennee@linaro.org> wrote:
> >> Can't be added as a subregion to the container...
> >>
> >>   qemu-system-arm: ../../softmmu/memory.c:2538: memory_region_add_subregion_common: Assertion `!subregion->container' failed.
> >
> > This assert means you tried to add the same MemoryRegion
> > as a subregion of more than one parent MR.
>
> Right - that is probably something we should make (more?) explicitly
> clear in the Memory API docs.

The doc comment does document the requirement:
 * [...] A region
 * may only be added once as a subregion (unless removed with
 * memory_region_del_subregion()); use memory_region_init_alias() if you
 * want a region to be a subregion in multiple locations.

One of the deficiencies of C assert() is the lack of an
explanatory text message to go along with the raw expression.

> > You can either:
> >  * pass all the CPUs the same container as their "memory" link,
> >    if they all see the same view of the world
>
> This should be the case - I don't think the different cores have any
> particular different view of the world. The use of the two 4kb banks I
> think is purely convention.
>
> However trying for a single container shared between both cores fails
> because armv7m_realize adds it's board_memory to another container:
>
>     memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);

Yeah, that trick only works for the real CPU object, not for
passing to SoC or SoC-like objects.

> So I guess I just have to repeat the creation of the aliases for each
> core. This seems needlessly messy...

It's not great, but the MR tree does need each MR to have only
one parent. Maybe there's some way to make it a bit less tedious
to create the aliases.

-- PMM


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Trying to understand QOM object creation and property linking
  2022-01-06 15:04         ` Peter Maydell
@ 2022-01-06 15:44           ` Alex Bennée
  2022-01-06 15:52             ` Peter Maydell
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2022-01-06 15:44 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Eduardo Habkost, Paolo Bonzini, Daniel P. Berrange,
	Philippe Mathieu-Daudé,
	qemu-devel


Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 6 Jan 2022 at 14:26, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > On Wed, 5 Jan 2022 at 21:05, Alex Bennée <alex.bennee@linaro.org> wrote:
>> >> Can't be added as a subregion to the container...
>> >>
>> >>   qemu-system-arm: ../../softmmu/memory.c:2538: memory_region_add_subregion_common: Assertion `!subregion->container' failed.
>> >
>> > This assert means you tried to add the same MemoryRegion
>> > as a subregion of more than one parent MR.
>>
>> Right - that is probably something we should make (more?) explicitly
>> clear in the Memory API docs.
>
> The doc comment does document the requirement:
>  * [...] A region
>  * may only be added once as a subregion (unless removed with
>  * memory_region_del_subregion()); use memory_region_init_alias() if you
>  * want a region to be a subregion in multiple locations.
>
> One of the deficiencies of C assert() is the lack of an
> explanatory text message to go along with the raw expression.
>
>> > You can either:
>> >  * pass all the CPUs the same container as their "memory" link,
>> >    if they all see the same view of the world
>>
>> This should be the case - I don't think the different cores have any
>> particular different view of the world. The use of the two 4kb banks I
>> think is purely convention.
>>
>> However trying for a single container shared between both cores fails
>> because armv7m_realize adds it's board_memory to another container:
>>
>>     memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);
>
> Yeah, that trick only works for the real CPU object, not for
> passing to SoC or SoC-like objects.

Hmm I wonder if I should be instantiating the underlying CPU object?
AIUI the cores are cortex-m0+ so I assume that comes with the gic/irq
wiring up that armv7m does?

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Trying to understand QOM object creation and property linking
  2022-01-06 15:44           ` Alex Bennée
@ 2022-01-06 15:52             ` Peter Maydell
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2022-01-06 15:52 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Eduardo Habkost, Paolo Bonzini, Daniel P. Berrange,
	Philippe Mathieu-Daudé,
	qemu-devel

On Thu, 6 Jan 2022 at 15:45, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> > Yeah, that trick only works for the real CPU object, not for
> > passing to SoC or SoC-like objects.
>
> Hmm I wonder if I should be instantiating the underlying CPU object?
> AIUI the cores are cortex-m0+ so I assume that comes with the gic/irq
> wiring up that armv7m does?

M-profile's a funny special case -- you do always need to create
the armv7m object, which wires up the NVIC and CPU objects together.

-- PMM


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-01-06 15:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-05 18:03 Trying to understand QOM object creation and property linking Alex Bennée
2022-01-05 19:03 ` Philippe Mathieu-Daudé
2022-01-05 21:02   ` Alex Bennée
2022-01-06 11:16     ` Peter Maydell
2022-01-06 14:20       ` Alex Bennée
2022-01-06 15:04         ` Peter Maydell
2022-01-06 15:44           ` Alex Bennée
2022-01-06 15:52             ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.