On Sat, Jul 02, 2022 at 08:01:14AM +0200, Cédric Le Goater wrote:
> On 7/1/22 20:06, Peter Delevoryas wrote:
> > To support multiple SoC's running simultaneously, we need a unique name for
> > each RAM region. DRAM is created by the machine, but SRAM is created by the
> > SoC, since in hardware it is part of the SoC's internals.
> >
> > We need a way to uniquely identify each SRAM region though, for VM
> > migration. Since each of the SoC's CPU's has an index which identifies it
> > uniquely from other CPU's in the machine, we can use the index of any of the
> > CPU's in the SoC to uniquely identify differentiate the SRAM name from other
> > SoC SRAM's. In this change, I just elected to use the index of the first CPU
> > in each SoC.
>
> hopefully the index is allocated. Did you check ?
You mean the CpuState.cpu_index? I think it's allocated at this point, I
actually had to do some debugging just to get it working cause I typo'd the
CPU(...) cast at first. I also tried it with the multi-SoC board in your
aspeed-7.1 branch:
(qemu) qom-get /machine/bmc aspeed.sram.0[0]
"/machine/bmc/aspeed.sram.0[0]"
(qemu) qom-get /machine/unattached aspeed.sram.2[0]
"/machine/unattached/aspeed.sram.2[0]"
I think the SRAM in the ast1030 is initialized without a parent object
(memory_region_init_ram(..., NULL, ...)) so that's why it's in the unattached
area. But we could fix that, maybe I should send a v2 with that too?
>
>
> > Signed-off-by: Peter Delevoryas <m...@pjd.dev>
> > ---
> > hw/arm/aspeed_ast10x0.c | 5 ++++-
> > hw/arm/aspeed_ast2600.c | 5 +++--
> > hw/arm/aspeed_soc.c | 5 +++--
> > 3 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> > index 33ef331771..b6b6f0d053 100644
> > --- a/hw/arm/aspeed_ast10x0.c
> > +++ b/hw/arm/aspeed_ast10x0.c
> > @@ -159,6 +159,7 @@ static void aspeed_soc_ast1030_realize(DeviceState
> > *dev_soc, Error **errp)
> > DeviceState *armv7m;
> > Error *err = NULL;
> > int i;
> > + char name[64];
> > if (!clock_has_source(s->sysclk)) {
> > error_setg(errp, "sysclk clock must be wired up by the board
> > code");
> > @@ -183,7 +184,9 @@ static void aspeed_soc_ast1030_realize(DeviceState
> > *dev_soc, Error **errp)
> > sysbus_realize(SYS_BUS_DEVICE(&s->armv7m), &error_abort);
> > /* Internal SRAM */
> > - memory_region_init_ram(&s->sram, NULL, "aspeed.sram", sc->sram_size,
> > &err);
> > + snprintf(name, sizeof(name), "aspeed.sram.%d",
> > + CPU(s->armv7m.cpu)->cpu_index);
> > + memory_region_init_ram(&s->sram, NULL, name, sc->sram_size, &err);
> > if (err != NULL) {
> > error_propagate(errp, err);
> > return;
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index 3f0611ac11..7efb9f888a 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -276,6 +276,7 @@ static void aspeed_soc_ast2600_realize(DeviceState
> > *dev, Error **errp)
> > AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> > Error *err = NULL;
> > qemu_irq irq;
> > + char name[64];
>
> May be ?
>
> g_autofree char *sram_name =
> g_strdup_printf("aspeed.sram.%d", CPU(&s->cpu[0])->cpu_index);
Hmmm yeah sure why not, I can fix the unattached AST1030 SRAM too. I always
wanted to use g_autofree some day hehe.
>
>
> Thanks,
>
> C.
>
>
> > /* IO space */
> > aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem),
> > "aspeed.io",
> > @@ -335,8 +336,8 @@ static void aspeed_soc_ast2600_realize(DeviceState
> > *dev, Error **errp)
> > }
> > /* SRAM */
> > - memory_region_init_ram(&s->sram, OBJECT(dev), "aspeed.sram",
> > - sc->sram_size, &err);
> > + snprintf(name, sizeof(name), "aspeed.sram.%d",
> > CPU(&s->cpu[0])->cpu_index);
> > + memory_region_init_ram(&s->sram, OBJECT(dev), name, sc->sram_size,
> > &err);
> > if (err) {
> > error_propagate(errp, err);
> > return;
> > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> > index 0f675e7fcd..1ddba33d2a 100644
> > --- a/hw/arm/aspeed_soc.c
> > +++ b/hw/arm/aspeed_soc.c
> > @@ -239,6 +239,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error
> > **errp)
> > AspeedSoCState *s = ASPEED_SOC(dev);
> > AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> > Error *err = NULL;
> > + char name[64];
> > /* IO space */
> > aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem),
> > "aspeed.io",
> > @@ -259,8 +260,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error
> > **errp)
> > }
> > /* SRAM */
> > - memory_region_init_ram(&s->sram, OBJECT(dev), "aspeed.sram",
> > - sc->sram_size, &err);
> > + snprintf(name, sizeof(name), "aspeed.sram.%d",
> > CPU(&s->cpu[0])->cpu_index);
> > + memory_region_init_ram(&s->sram, OBJECT(dev), name, sc->sram_size,
> > &err);
> > if (err) {
> > error_propagate(errp, err);
> > return;
>
