Re: [RFC PATCH 2/3] hw/block/pflash: Move code around

2021-02-26 Thread David Edmondson
On Friday, 2021-02-26 at 00:02:37 +01, Philippe Mathieu-Daudé wrote:

> First do the block checks, so we know if it is read-only or not.
> Then create the MemoryRegion. This will allow optimization in
> the next commit.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Edmondson 

> ---
>  hw/block/pflash_cfi01.c | 24 
>  hw/block/pflash_cfi02.c | 18 +-
>  2 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 22287a1522e..a5fa8d8b74a 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -731,18 +731,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
> **errp)
>  }
>  device_len = sector_len_per_device * blocks_per_device;
>  
> -memory_region_init_rom_device(
> -&pfl->mem, OBJECT(dev),
> -&pflash_cfi01_ops,
> -pfl,
> -pfl->name, total_len, errp);
> -if (*errp) {
> -return;
> -}
> -
> -pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
> -sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
> -
>  if (pfl->blk) {
>  uint64_t perm;
>  pfl->ro = !blk_supports_write_perm(pfl->blk);
> @@ -755,6 +743,18 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
> **errp)
>  pfl->ro = 0;
>  }
>  
> +memory_region_init_rom_device(
> +&pfl->mem, OBJECT(dev),
> +&pflash_cfi01_ops,
> +pfl,
> +pfl->name, total_len, errp);
> +if (*errp) {
> +return;
> +}
> +
> +pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
> +sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
> +
>  if (pfl->blk) {
>  if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, total_len,
>   errp)) {
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 7962cff7455..4f62ce8917d 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -791,15 +791,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
> **errp)
>  return;
>  }
>  
> -memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
> -  &pflash_cfi02_ops, pfl, pfl->name,
> -  pfl->chip_len, errp);
> -if (*errp) {
> -return;
> -}
> -
> -pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
> -
>  if (pfl->blk) {
>  uint64_t perm;
>  pfl->ro = !blk_supports_write_perm(pfl->blk);
> @@ -812,6 +803,15 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
> **errp)
>  pfl->ro = 0;
>  }
>  
> +memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
> +  &pflash_cfi02_ops, pfl, pfl->name,
> +  pfl->chip_len, errp);
> +if (*errp) {
> +return;
> +}
> +
> +pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
> +
>  if (pfl->blk) {
>  if (!blk_check_size_and_read_all(pfl->blk, pfl->storage,
>   pfl->chip_len, errp)) {
> -- 
> 2.26.2

dme.
-- 
She's as sweet as Tupelo honey, she's an angel of the first degree.



[RFC PATCH 2/3] hw/block/pflash: Move code around

2021-02-25 Thread Philippe Mathieu-Daudé
First do the block checks, so we know if it is read-only or not.
Then create the MemoryRegion. This will allow optimization in
the next commit.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi01.c | 24 
 hw/block/pflash_cfi02.c | 18 +-
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 22287a1522e..a5fa8d8b74a 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -731,18 +731,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
**errp)
 }
 device_len = sector_len_per_device * blocks_per_device;
 
-memory_region_init_rom_device(
-&pfl->mem, OBJECT(dev),
-&pflash_cfi01_ops,
-pfl,
-pfl->name, total_len, errp);
-if (*errp) {
-return;
-}
-
-pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
-sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
-
 if (pfl->blk) {
 uint64_t perm;
 pfl->ro = !blk_supports_write_perm(pfl->blk);
@@ -755,6 +743,18 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
**errp)
 pfl->ro = 0;
 }
 
+memory_region_init_rom_device(
+&pfl->mem, OBJECT(dev),
+&pflash_cfi01_ops,
+pfl,
+pfl->name, total_len, errp);
+if (*errp) {
+return;
+}
+
+pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
+sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
+
 if (pfl->blk) {
 if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, total_len,
  errp)) {
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 7962cff7455..4f62ce8917d 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -791,15 +791,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
-  &pflash_cfi02_ops, pfl, pfl->name,
-  pfl->chip_len, errp);
-if (*errp) {
-return;
-}
-
-pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
-
 if (pfl->blk) {
 uint64_t perm;
 pfl->ro = !blk_supports_write_perm(pfl->blk);
@@ -812,6 +803,15 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
**errp)
 pfl->ro = 0;
 }
 
+memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
+  &pflash_cfi02_ops, pfl, pfl->name,
+  pfl->chip_len, errp);
+if (*errp) {
+return;
+}
+
+pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
+
 if (pfl->blk) {
 if (!blk_check_size_and_read_all(pfl->blk, pfl->storage,
  pfl->chip_len, errp)) {
-- 
2.26.2