| Message ID | 20180420124835.7268-3-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | aspeed, highbank: fix RAM migration oddities | expand |
On 04/20/2018 02:48 PM, Peter Maydell wrote: > Currently we use memory_region_init_ram_nomigrate() to create > the "aspeed.boot_rom" memory region, and we don't manually > register it with vmstate_register_ram(). This currently > means that its contents are migrated but as a ram block > whose name is the empty string; in future it may mean they > are not migrated at all. Use memory_region_init_ram() instead. > > Note that this is a cross-version migration compatibility break > for the "palmetto-bmc", "ast2500-evb" and "romulus-bmc" machines. Migration does not work for these. > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Tested-by: Cédric Le Goater <clg@kaod.org> On palmetto, romulus and ast2500-evb boards Thanks C. > --- > hw/arm/aspeed.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 7088c907bd..aecb3c1e75 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -225,7 +225,7 @@ static void aspeed_board_init(MachineState *machine, > * SoC and 128MB for the AST2500 SoC, which is twice as big as > * needed by the flash modules of the Aspeed machines. > */ > - memory_region_init_rom_nomigrate(boot_rom, OBJECT(bmc), "aspeed.boot_rom", > + memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom", > fl->size, &error_abort); > memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR, > boot_rom); >
On 21 April 2018 at 10:07, Cédric Le Goater <clg@kaod.org> wrote: > On 04/20/2018 02:48 PM, Peter Maydell wrote: >> Currently we use memory_region_init_ram_nomigrate() to create >> the "aspeed.boot_rom" memory region, and we don't manually >> register it with vmstate_register_ram(). This currently >> means that its contents are migrated but as a ram block >> whose name is the empty string; in future it may mean they >> are not migrated at all. Use memory_region_init_ram() instead. >> >> Note that this is a cross-version migration compatibility break >> for the "palmetto-bmc", "ast2500-evb" and "romulus-bmc" machines. > > Migration does not work for these. Well, at least we're not breaking anything :-) Do you know why migration doesn't work? thanks -- PMM
On 04/21/2018 11:34 AM, Peter Maydell wrote: > On 21 April 2018 at 10:07, Cédric Le Goater <clg@kaod.org> wrote: >> On 04/20/2018 02:48 PM, Peter Maydell wrote: >>> Currently we use memory_region_init_ram_nomigrate() to create >>> the "aspeed.boot_rom" memory region, and we don't manually >>> register it with vmstate_register_ram(). This currently >>> means that its contents are migrated but as a ram block >>> whose name is the empty string; in future it may mean they >>> are not migrated at all. Use memory_region_init_ram() instead. >>> >>> Note that this is a cross-version migration compatibility break >>> for the "palmetto-bmc", "ast2500-evb" and "romulus-bmc" machines. >> >> Migration does not work for these. > > Well, at least we're not breaking anything :-) > Do you know why migration doesn't work? A quick migration test shows that : qemu-system-arm: Missing section footer for aspeed.timerctrl qemu-system-arm: load of migration failed: Invalid argument which is surprising, the state looks correct. I will dig in. Thanks, C. "aspeed.timerctrl (8)": { "ctrl": "0x00000003", "ctrl2": "0x00000000", "timers": [ { "id": "0x00", "level": "0x00000000", "timer": "00 00 00 29 aa bd 88 44", "reload": "0xffffffff", "match": [ "0x00000000", "0x00000000" ] }, { "id": "0x01", "level": "0x00000000", "timer": "ff ff ff ff ff ff ff ff", "reload": "0x00000000", "match": [ "0x00000000", "0x00000000" ] }, { "id": "0x02", "level": "0x00000000", "timer": "ff ff ff ff ff ff ff ff", "reload": "0x00000000", "match": [ "0x00000000", "0x00000000" ] }, { "id": "0x03", "level": "0x00000000", "timer": "ff ff ff ff ff ff ff ff", "reload": "0xffffffff", "match": [ "0x00000000", "0x00000000" ] }, { "id": "0x04", "level": "0x00000000", "timer": "ff ff ff ff ff ff ff ff", "reload": "0x00000000", "match": [ "0x00000000", "0x00000000" ] }, { "id": "0x05", "level": "0x00000000", "timer": "ff ff ff ff ff ff ff ff", "reload": "0x00000000", "match": [ "0x00000000", "0x00000000" ] }, { "id": "0x06", "level": "0x00000000", "timer": "ff ff ff ff ff ff ff ff", "reload": "0x00000000", "match": [ "0x00000000", "0x00000000" ] }, { "id": "0x07", "level": "0x00000000", "timer": "ff ff ff ff ff ff ff ff", "reload": "0x00000000", "match": [ "0x00000000", "0x00000000" ] } ] },
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 7088c907bd..aecb3c1e75 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -225,7 +225,7 @@ static void aspeed_board_init(MachineState *machine, * SoC and 128MB for the AST2500 SoC, which is twice as big as * needed by the flash modules of the Aspeed machines. */ - memory_region_init_rom_nomigrate(boot_rom, OBJECT(bmc), "aspeed.boot_rom", + memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom", fl->size, &error_abort); memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR, boot_rom);
Currently we use memory_region_init_ram_nomigrate() to create the "aspeed.boot_rom" memory region, and we don't manually register it with vmstate_register_ram(). This currently means that its contents are migrated but as a ram block whose name is the empty string; in future it may mean they are not migrated at all. Use memory_region_init_ram() instead. Note that this is a cross-version migration compatibility break for the "palmetto-bmc", "ast2500-evb" and "romulus-bmc" machines. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.17.0