[Qemu-devel] [PATCH 0/2] mips boston/malta: don't have nomigrate RAM regions

[Qemu-devel] [PATCH 0/2] mips boston/malta: don't have nomigrate RAM regions

[Peter Maydell]

This patchset fixes a a bug in the MIPS boston and malta boards:
they currently use memory_region_init_rom_nomigrate() to create
memory regions for their BIOS/flash, and they don't manually
register the MR with vmstate_register_ram() either. This currently
means that its contents are migrated, but as a RAM block whose
name is the empty string; in future (when a patch which is currently
in the outstanding migration tree pull request hits master) it
will mean they are not migrated at all. Switch to using
memory_region_init_ram() instead, so the memory contents are
migrated with a reasonable name.

Note that this is a cross-version migration compatibility break
for both machines, but:
 (a) migration will break anyway due to the "don't migrate
     unnamed RAM blocks" patch
 (b) neither machine is versioned, so we don't really care about
     maintaining cross-version migration compatibility AFAIK

thanks
-- PMM

Peter Maydell (2):
  hw/mips/boston: don't make flash region 'nomigrate'
  hw/mips/mips_malta: don't make bios region 'nomigrate'

 hw/mips/boston.c     | 3 +--
 hw/mips/mips_malta.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/2] hw/mips/boston: don't make flash region 'nomigrate'

[Peter Maydell]

Currently we use memory_region_init_rom_nomigrate() to create
the "boston.flash" 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 a cross-version migration compatibility break
for the "boston" machine.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/mips/boston.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 5302e5c885..1d136529ad 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -471,8 +471,7 @@ static void boston_mach_init(MachineState *machine)
     sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->cps), 0, 0, 1);
 
     flash =  g_new(MemoryRegion, 1);
-    memory_region_init_rom_nomigrate(flash, NULL,
-                                     "boston.flash", 128 * M_BYTE, &err);
+    memory_region_init_rom(flash, NULL, "boston.flash", 128 * M_BYTE, &err);
     memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
 
     ddr = g_new(MemoryRegion, 1);
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/2] hw/mips/boston: don't make flash region 'nomigrate'

[Cédric Le Goater]

On 06/04/2018 01:03 PM, Peter Maydell wrote:
> Currently we use memory_region_init_rom_nomigrate() to create
> the "boston.flash" 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 a cross-version migration compatibility break
> for the "boston" machine.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/mips/boston.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index 5302e5c885..1d136529ad 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -471,8 +471,7 @@ static void boston_mach_init(MachineState *machine)
>      sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->cps), 0, 0, 1);
>  
>      flash =  g_new(MemoryRegion, 1);
> -    memory_region_init_rom_nomigrate(flash, NULL,
> -                                     "boston.flash", 128 * M_BYTE, &err);
> +    memory_region_init_rom(flash, NULL, "boston.flash", 128 * M_BYTE, &err);
>      memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
>  
>      ddr = g_new(MemoryRegion, 1);
> 

Re: [Qemu-devel] [PATCH 1/2] hw/mips/boston: don't make flash region 'nomigrate'

[Philippe Mathieu-Daudé]

On 06/04/2018 08:03 AM, Peter Maydell wrote:
> Currently we use memory_region_init_rom_nomigrate() to create
> the "boston.flash" 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 a cross-version migration compatibility break
> for the "boston" machine.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

> ---
>  hw/mips/boston.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index 5302e5c885..1d136529ad 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -471,8 +471,7 @@ static void boston_mach_init(MachineState *machine)
>      sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->cps), 0, 0, 1);
>  
>      flash =  g_new(MemoryRegion, 1);
> -    memory_region_init_rom_nomigrate(flash, NULL,
> -                                     "boston.flash", 128 * M_BYTE, &err);
> +    memory_region_init_rom(flash, NULL, "boston.flash", 128 * M_BYTE, &err);
>      memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
>  
>      ddr = g_new(MemoryRegion, 1);
> 

[Qemu-devel] [PATCH 2/2] hw/mips/mips_malta: don't make bios region 'nomigrate'

[Peter Maydell]

Currently we use memory_region_init_rom_nomigrate() to create
the "bios.1fc" 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 a cross-version migration compatibility break
for the "malta" machine.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/mips/mips_malta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index af70ecffc0..927944899b 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1153,7 +1153,7 @@ void mips_malta_init(MachineState *machine)
      * handled by an overlapping region as the resulting ROM code subpage
      * regions are not executable.
      */
-    memory_region_init_ram_nomigrate(bios_copy, NULL, "bios.1fc", BIOS_SIZE,
+    memory_region_init_ram(bios_copy, NULL, "bios.1fc", BIOS_SIZE,
                            &error_fatal);
     if (!rom_copy(memory_region_get_ram_ptr(bios_copy),
                   FLASH_ADDRESS, BIOS_SIZE)) {
-- 
2.17.1

Re: [Qemu-devel] [PATCH 2/2] hw/mips/mips_malta: don't make bios region 'nomigrate'

[Cédric Le Goater]

On 06/04/2018 01:03 PM, Peter Maydell wrote:
> Currently we use memory_region_init_rom_nomigrate() to create
> the "bios.1fc" 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 a cross-version migration compatibility break
> for the "malta" machine.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/mips/mips_malta.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index af70ecffc0..927944899b 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1153,7 +1153,7 @@ void mips_malta_init(MachineState *machine)
>       * handled by an overlapping region as the resulting ROM code subpage
>       * regions are not executable.
>       */
> -    memory_region_init_ram_nomigrate(bios_copy, NULL, "bios.1fc", BIOS_SIZE,
> +    memory_region_init_ram(bios_copy, NULL, "bios.1fc", BIOS_SIZE,
>                             &error_fatal);
>      if (!rom_copy(memory_region_get_ram_ptr(bios_copy),
>                    FLASH_ADDRESS, BIOS_SIZE)) {
> 

Re: [Qemu-devel] [PATCH 2/2] hw/mips/mips_malta: don't make bios region 'nomigrate'

[Philippe Mathieu-Daudé]

On 06/04/2018 08:03 AM, Peter Maydell wrote:
> Currently we use memory_region_init_rom_nomigrate() to create
> the "bios.1fc" 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 a cross-version migration compatibility break
> for the "malta" machine.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

> ---
>  hw/mips/mips_malta.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index af70ecffc0..927944899b 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1153,7 +1153,7 @@ void mips_malta_init(MachineState *machine)
>       * handled by an overlapping region as the resulting ROM code subpage
>       * regions are not executable.
>       */
> -    memory_region_init_ram_nomigrate(bios_copy, NULL, "bios.1fc", BIOS_SIZE,
> +    memory_region_init_ram(bios_copy, NULL, "bios.1fc", BIOS_SIZE,
>                             &error_fatal);
>      if (!rom_copy(memory_region_get_ram_ptr(bios_copy),
>                    FLASH_ADDRESS, BIOS_SIZE)) {
> 

Re: [Qemu-devel] [PATCH 0/2] mips boston/malta: don't have nomigrate RAM regions

[Peter Maydell]

On 4 June 2018 at 12:03, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset fixes a a bug in the MIPS boston and malta boards:
> they currently use memory_region_init_rom_nomigrate() to create
> memory regions for their BIOS/flash, and they don't manually
> register the MR with vmstate_register_ram() either. This currently
> means that its contents are migrated, but as a RAM block whose
> name is the empty string; in future (when a patch which is currently
> in the outstanding migration tree pull request hits master) it
> will mean they are not migrated at all. Switch to using
> memory_region_init_ram() instead, so the memory contents are
> migrated with a reasonable name.
>
> Note that this is a cross-version migration compatibility break
> for both machines, but:
>  (a) migration will break anyway due to the "don't migrate
>      unnamed RAM blocks" patch
>  (b) neither machine is versioned, so we don't really care about
>      maintaining cross-version migration compatibility AFAIK

Any comment from the MIPS maintainers?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/2] mips boston/malta: don't have nomigrate RAM regions

[Paul Burton]

Hi Peter,

On Fri, Jun 15, 2018 at 11:08:11AM +0100, Peter Maydell wrote:
> On 4 June 2018 at 12:03, Peter Maydell <peter.maydell@linaro.org> wrote:
> > This patchset fixes a a bug in the MIPS boston and malta boards:
> > they currently use memory_region_init_rom_nomigrate() to create
> > memory regions for their BIOS/flash, and they don't manually
> > register the MR with vmstate_register_ram() either. This currently
> > means that its contents are migrated, but as a RAM block whose
> > name is the empty string; in future (when a patch which is currently
> > in the outstanding migration tree pull request hits master) it
> > will mean they are not migrated at all. Switch to using
> > memory_region_init_ram() instead, so the memory contents are
> > migrated with a reasonable name.
> >
> > Note that this is a cross-version migration compatibility break
> > for both machines, but:
> >  (a) migration will break anyway due to the "don't migrate
> >      unnamed RAM blocks" patch
> >  (b) neither machine is versioned, so we don't really care about
> >      maintaining cross-version migration compatibility AFAIK
> 
> Any comment from the MIPS maintainers?
> 
> thanks
> -- PMM

FWIW, this looks fine to me for both Boston & Malta:

    Reviewed-by: Paul Burton <paul.burton@mips.com>

Thanks,
    Paul