[PATCH-for-6.1 0/2] hw/block/pflash_cfi02: Do not create aliases when not necessary

[PATCH-for-6.1 0/2] hw/block/pflash_cfi02: Do not create aliases when not necessary

[Philippe Mathieu-Daudé]

Simplify memory layout when no pflash_cfi02 mapping requested.
For example using the r2d machine:

  (qemu) info mtree
  address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      0000000000000000-0000000000ffffff (prio 0, i/o): pflash
        0000000000000000-0000000000ffffff (prio 0, romd): alias pflash-alias =
@r2d.flash 0000000000000000-0000000000ffffff
      0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
      000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram

  (qemu) info mtree
  address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      0000000000000000-0000000000ffffff (prio 0, romd): r2d.flash
      0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
      000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram

Philippe Mathieu-Daud=C3=A9 (2):
  hw/block/pflash_cfi02: Set romd mode in pflash_cfi02_realize()
  hw/block/pflash_cfi02: Do not create aliases when not necessary

 hw/block/pflash_cfi02.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--=20
2.26.2

[PATCH-for-6.1 1/2] hw/block/pflash_cfi02: Set romd mode in pflash_cfi02_realize()

[Philippe Mathieu-Daudé]

The ROMD mode isn't related to mapping setup.
Ideally we'd set this mode when the state machine resets,
but for now simply move it to pflash_cfi02_realize() to
not introduce logical change.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/block/pflash_cfi02.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 25c053693ce..35e30bb812c 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -173,7 +173,6 @@ static void pflash_setup_mappings(PFlashCFI02 *pfl)
                                  "pflash-alias", &pfl->orig_mem, 0, size);
         memory_region_add_subregion(&pfl->mem, i * size, &pfl->mem_mappings[i]);
     }
-    pfl->rom_mode = true;
 }
 
 static void pflash_reset_state_machine(PFlashCFI02 *pfl)
@@ -917,6 +916,7 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     /* Allocate memory for a bitmap for sectors being erased. */
     pfl->sector_erase_map = bitmap_new(pfl->total_sectors);
 
+    pfl->rom_mode = true;
     pflash_setup_mappings(pfl);
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
 
-- 
2.26.2

[PATCH-for-6.1 2/2] hw/block/pflash_cfi02: Do not create aliases when not necessary

[Philippe Mathieu-Daudé]

When no mapping is requested, it is pointless to create
alias regions.
Only create them when multiple mappings are requested to
simplify the memory layout. The flatview is not changed.

For example using 'qemu-system-sh4 -M r2d -S -monitor stdio',

* before:

  (qemu) info mtree
  address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      0000000000000000-0000000000ffffff (prio 0, i/o): pflash
        0000000000000000-0000000000ffffff (prio 0, romd): alias pflash-alias @r2d.flash 0000000000000000-0000000000ffffff
      0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
      000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram
  (qemu) info mtree -f
  FlatView #0
   AS "memory", root: system
   AS "cpu-memory-0", root: system
   Root memory region: system
    0000000000000000-0000000000ffffff (prio 0, romd): r2d.flash
    0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
    000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram

* after:

  (qemu) info mtree
  address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      0000000000000000-0000000000ffffff (prio 0, romd): r2d.flash
      0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
      000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram
  (qemu) info mtree -f
  FlatView #0
   AS "memory", root: system
   AS "cpu-memory-0", root: system
   Root memory region: system
    0000000000000000-0000000000ffffff (prio 0, romd): r2d.flash
    0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
    000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/block/pflash_cfi02.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 35e30bb812c..02c514fb6e0 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -917,8 +917,12 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     pfl->sector_erase_map = bitmap_new(pfl->total_sectors);
 
     pfl->rom_mode = true;
-    pflash_setup_mappings(pfl);
-    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
+    if (pfl->mappings > 1) {
+        pflash_setup_mappings(pfl);
+        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
+    } else {
+        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->orig_mem);
+    }
 
     timer_init_ns(&pfl->timer, QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
     pfl->status = 0;
-- 
2.26.2

Re: [PATCH-for-6.1 1/2] hw/block/pflash_cfi02: Set romd mode in pflash_cfi02_realize()

[Richard Henderson]

On 3/25/21 6:09 AM, Philippe Mathieu-Daudé wrote:
> The ROMD mode isn't related to mapping setup.
> Ideally we'd set this mode when the state machine resets,
> but for now simply move it to pflash_cfi02_realize() to
> not introduce logical change.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   hw/block/pflash_cfi02.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH-for-6.1 2/2] hw/block/pflash_cfi02: Do not create aliases when not necessary

[Richard Henderson]

On 3/25/21 6:09 AM, Philippe Mathieu-Daudé wrote:
> When no mapping is requested, it is pointless to create
> alias regions.
> Only create them when multiple mappings are requested to
> simplify the memory layout. The flatview is not changed.
> 
> For example using 'qemu-system-sh4 -M r2d -S -monitor stdio',
> 
> * before:
> 
>    (qemu) info mtree
>    address-space: memory
>      0000000000000000-ffffffffffffffff (prio 0, i/o): system
>        0000000000000000-0000000000ffffff (prio 0, i/o): pflash
>          0000000000000000-0000000000ffffff (prio 0, romd): alias pflash-alias @r2d.flash 0000000000000000-0000000000ffffff
>        0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
>        000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram
>    (qemu) info mtree -f
>    FlatView #0
>     AS "memory", root: system
>     AS "cpu-memory-0", root: system
>     Root memory region: system
>      0000000000000000-0000000000ffffff (prio 0, romd): r2d.flash
>      0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
>      000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram
> 
> * after:
> 
>    (qemu) info mtree
>    address-space: memory
>      0000000000000000-ffffffffffffffff (prio 0, i/o): system
>        0000000000000000-0000000000ffffff (prio 0, romd): r2d.flash
>        0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
>        000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram
>    (qemu) info mtree -f
>    FlatView #0
>     AS "memory", root: system
>     AS "cpu-memory-0", root: system
>     Root memory region: system
>      0000000000000000-0000000000ffffff (prio 0, romd): r2d.flash
>      0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
>      000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/block/pflash_cfi02.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> 
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 35e30bb812c..02c514fb6e0 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -917,8 +917,12 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
>       pfl->sector_erase_map = bitmap_new(pfl->total_sectors);
>   
>       pfl->rom_mode = true;
> -    pflash_setup_mappings(pfl);
> -    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
> +    if (pfl->mappings > 1) {
> +        pflash_setup_mappings(pfl);
> +        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
> +    } else {
> +        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->orig_mem);
> +    }
>   
>       timer_init_ns(&pfl->timer, QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
>       pfl->status = 0;
> 

Re: [PATCH-for-6.1 0/2] hw/block/pflash_cfi02: Do not create aliases when not necessary

[Philippe Mathieu-Daudé]

On 3/25/21 1:09 PM, Philippe Mathieu-Daudé wrote:
> Simplify memory layout when no pflash_cfi02 mapping requested.

> Philippe Mathieu-Daud=C3=A9 (2):
>   hw/block/pflash_cfi02: Set romd mode in pflash_cfi02_realize()
>   hw/block/pflash_cfi02: Do not create aliases when not necessary

Thanks, series applied to pflash-next tree.