[v2] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number

[v2] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number

[Tim Lee]

Fix flash device part number to `mx66l1g45g` according image-bmc run on npcm8xx
evb board (SPIFlash...SF: Detected mx66l1g45g, total 128 MiB)

And add auto zero flash image size to resolve error below after executing
`./qemu-system-aarch64 -machine npcm845-evb -drive file=image-bmc`

Error message:
qemu-system-aarch64: mx66l1g45g device '/machine/unattached/device[73]'
requires 134217728 bytes, mtd0 block backend provides 67108864 bytes

Tested:
Build passes and runs ./qemu-system-aarch64 -machine npcm845-evb normally

Signed-off-by: Tim Lee <timlee660101@gmail.com>
---
Changes since v1:
- Add a statement that checks whether the storage is writable

 hw/arm/npcm8xx_boards.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c
index 3fb8478e72..79295a586c 100644
--- a/hw/arm/npcm8xx_boards.c
+++ b/hw/arm/npcm8xx_boards.c
@@ -27,6 +27,7 @@
 #include "qemu/error-report.h"
 #include "qemu/datadir.h"
 #include "qemu/units.h"
+#include "system/block-backend.h"
 
 #define NPCM845_EVB_POWER_ON_STRAPS 0x000017ff
 
@@ -59,10 +60,26 @@ static void npcm8xx_connect_flash(NPCM7xxFIUState *fiu, int cs_no,
 {
     DeviceState *flash;
     qemu_irq flash_cs;
+    BlockBackend *blk;
+    BlockDriverState *bs;
+    uint64_t blk_size, perm, shared_perm;
 
     flash = qdev_new(flash_type);
     if (dinfo) {
         qdev_prop_set_drive(flash, "drive", blk_by_legacy_dinfo(dinfo));
+        blk = blk_by_legacy_dinfo(dinfo);
+        bs = blk_bs(blk);
+        blk_size = blk_getlength(blk);
+
+        if (!bdrv_is_read_only(bs)) {
+            if (blk_size < fiu->flash_size) {
+                blk_get_perm(blk, &perm, &shared_perm);
+                blk_set_perm(blk, BLK_PERM_ALL, BLK_PERM_ALL, &error_abort);
+                blk_truncate(blk, fiu->flash_size, true, PREALLOC_MODE_OFF,
+                             BDRV_REQ_ZERO_WRITE, &error_abort);
+                blk_set_perm(blk, perm, shared_perm, &error_abort);
+            }
+        }
     }
     qdev_realize_and_unref(flash, BUS(fiu->spi), &error_fatal);
 
@@ -194,7 +211,8 @@ static void npcm845_evb_init(MachineState *machine)
     qdev_realize(DEVICE(soc), NULL, &error_fatal);
 
     npcm8xx_load_bootrom(machine, soc);
-    npcm8xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0, 0));
+    npcm8xx_connect_flash(&soc->fiu[0], 0, "mx66l1g45g",
+                          drive_get(IF_MTD, 0, 0));
     npcm845_evb_i2c_init(soc);
     npcm845_evb_fan_init(NPCM8XX_MACHINE(machine), soc);
     npcm8xx_load_kernel(machine, soc);
-- 
2.34.1

Re: [v2] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number

[Philippe Mathieu-Daudé]

Hi Tim,

On 8/5/25 04:15, Tim Lee wrote:
> Fix flash device part number to `mx66l1g45g` according image-bmc run on npcm8xx
> evb board (SPIFlash...SF: Detected mx66l1g45g, total 128 MiB)
> 
> And add auto zero flash image size to resolve error below after executing
> `./qemu-system-aarch64 -machine npcm845-evb -drive file=image-bmc`
> 
> Error message:
> qemu-system-aarch64: mx66l1g45g device '/machine/unattached/device[73]'
> requires 134217728 bytes, mtd0 block backend provides 67108864 bytes
> 
> Tested:
> Build passes and runs ./qemu-system-aarch64 -machine npcm845-evb normally
> 
> Signed-off-by: Tim Lee <timlee660101@gmail.com>
> ---
> Changes since v1:
> - Add a statement that checks whether the storage is writable
> 
>   hw/arm/npcm8xx_boards.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c
> index 3fb8478e72..79295a586c 100644
> --- a/hw/arm/npcm8xx_boards.c
> +++ b/hw/arm/npcm8xx_boards.c
> @@ -27,6 +27,7 @@
>   #include "qemu/error-report.h"
>   #include "qemu/datadir.h"
>   #include "qemu/units.h"
> +#include "system/block-backend.h"
>   
>   #define NPCM845_EVB_POWER_ON_STRAPS 0x000017ff
>   
> @@ -59,10 +60,26 @@ static void npcm8xx_connect_flash(NPCM7xxFIUState *fiu, int cs_no,
>   {
>       DeviceState *flash;
>       qemu_irq flash_cs;
> +    BlockBackend *blk;
> +    BlockDriverState *bs;
> +    uint64_t blk_size, perm, shared_perm;
>   
>       flash = qdev_new(flash_type);
>       if (dinfo) {
>           qdev_prop_set_drive(flash, "drive", blk_by_legacy_dinfo(dinfo));
> +        blk = blk_by_legacy_dinfo(dinfo);
> +        bs = blk_bs(blk);
> +        blk_size = blk_getlength(blk);
> +
> +        if (!bdrv_is_read_only(bs)) {

This isn't what I meant, we'll get the same issue with read-only storage.

See:
https://lore.kernel.org/qemu-devel/CAFEAcA9itEDTrznX1KRVEza__Dch95aBpPzbdTJ0-tuxwiHtoQ@mail.gmail.com/

> +            if (blk_size < fiu->flash_size) {
> +                blk_get_perm(blk, &perm, &shared_perm);
> +                blk_set_perm(blk, BLK_PERM_ALL, BLK_PERM_ALL, &error_abort);
> +                blk_truncate(blk, fiu->flash_size, true, PREALLOC_MODE_OFF,
> +                             BDRV_REQ_ZERO_WRITE, &error_abort);
> +                blk_set_perm(blk, perm, shared_perm, &error_abort);
> +            }
> +        }
>       }
>       qdev_realize_and_unref(flash, BUS(fiu->spi), &error_fatal);
>   
> @@ -194,7 +211,8 @@ static void npcm845_evb_init(MachineState *machine)
>       qdev_realize(DEVICE(soc), NULL, &error_fatal);
>   
>       npcm8xx_load_bootrom(machine, soc);
> -    npcm8xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0, 0));
> +    npcm8xx_connect_flash(&soc->fiu[0], 0, "mx66l1g45g",
> +                          drive_get(IF_MTD, 0, 0));
>       npcm845_evb_i2c_init(soc);
>       npcm845_evb_fan_init(NPCM8XX_MACHINE(machine), soc);
>       npcm8xx_load_kernel(machine, soc);

RE: [v2] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number

[KFTING]

From: Tim Lee <timlee660101@gmail.com>
Sent: Thursday, May 8, 2025 10:15 AM
To: peter.maydell@linaro.org; wuhaotsh@google.com; CS20 KFTing <KFTING@nuvoton.com>; CS20 CHLi30 <CHLI30@nuvoton.com>
Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; Tim Lee <timlee660101@gmail.com>
Subject: [v2] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number


Fix flash device part number to `mx66l1g45g` according image-bmc run on npcm8xx evb board (SPIFlash...SF: Detected mx66l1g45g, total 128 MiB)

And add auto zero flash image size to resolve error below after executing
`./qemu-system-aarch64 -machine npcm845-evb -drive file=image-bmc`

Error message:
qemu-system-aarch64: mx66l1g45g device '/machine/unattached/device[73]'
requires 134217728 bytes, mtd0 block backend provides 67108864 bytes

Tested:
Build passes and runs ./qemu-system-aarch64 -machine npcm845-evb normally

Signed-off-by: Tim Lee <timlee660101@gmail.com>
---
Changes since v1:
- Add a statement that checks whether the storage is writable

 hw/arm/npcm8xx_boards.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c index 3fb8478e72..79295a586c 100644
--- a/hw/arm/npcm8xx_boards.c
+++ b/hw/arm/npcm8xx_boards.c
@@ -27,6 +27,7 @@
 #include "qemu/error-report.h"
 #include "qemu/datadir.h"
 #include "qemu/units.h"
+#include "system/block-backend.h"

 #define NPCM845_EVB_POWER_ON_STRAPS 0x000017ff

@@ -59,10 +60,26 @@ static void npcm8xx_connect_flash(NPCM7xxFIUState *fiu, int cs_no,  {
     DeviceState *flash;
     qemu_irq flash_cs;
+    BlockBackend *blk;
+    BlockDriverState *bs;
+    uint64_t blk_size, perm, shared_perm;

     flash = qdev_new(flash_type);
     if (dinfo) {
         qdev_prop_set_drive(flash, "drive", blk_by_legacy_dinfo(dinfo));
+        blk = blk_by_legacy_dinfo(dinfo);
+        bs = blk_bs(blk);
+        blk_size = blk_getlength(blk);
+
+        if (!bdrv_is_read_only(bs)) {
+            if (blk_size < fiu->flash_size) {
+                blk_get_perm(blk, &perm, &shared_perm);
+                blk_set_perm(blk, BLK_PERM_ALL, BLK_PERM_ALL, &error_abort);
+                blk_truncate(blk, fiu->flash_size, true, PREALLOC_MODE_OFF,
+                             BDRV_REQ_ZERO_WRITE, &error_abort);
+                blk_set_perm(blk, perm, shared_perm, &error_abort);
+            }
+        }
     }
     qdev_realize_and_unref(flash, BUS(fiu->spi), &error_fatal);

@@ -194,7 +211,8 @@ static void npcm845_evb_init(MachineState *machine)
     qdev_realize(DEVICE(soc), NULL, &error_fatal);

     npcm8xx_load_bootrom(machine, soc);
-    npcm8xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0, 0));
+    npcm8xx_connect_flash(&soc->fiu[0], 0, "mx66l1g45g",
+                          drive_get(IF_MTD, 0, 0));
     npcm845_evb_i2c_init(soc);
     npcm845_evb_fan_init(NPCM8XX_MACHINE(machine), soc);
     npcm8xx_load_kernel(machine, soc);
--
2.34.1

Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
________________________________
________________________________
 The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.

Re: [v2] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number

[Tim Lee]

Philippe Mathieu-Daudé <philmd@linaro.org> 於 2025年5月8日 週四 下午2:19寫道：
>
> Hi Tim,
>
> On 8/5/25 04:15, Tim Lee wrote:
> > Fix flash device part number to `mx66l1g45g` according image-bmc run on npcm8xx
> > evb board (SPIFlash...SF: Detected mx66l1g45g, total 128 MiB)
> >
> > And add auto zero flash image size to resolve error below after executing
> > `./qemu-system-aarch64 -machine npcm845-evb -drive file=image-bmc`
> >
> > Error message:
> > qemu-system-aarch64: mx66l1g45g device '/machine/unattached/device[73]'
> > requires 134217728 bytes, mtd0 block backend provides 67108864 bytes
> >
> > Tested:
> > Build passes and runs ./qemu-system-aarch64 -machine npcm845-evb normally
> >
> > Signed-off-by: Tim Lee <timlee660101@gmail.com>
> > ---
> > Changes since v1:
> > - Add a statement that checks whether the storage is writable
> >
> >   hw/arm/npcm8xx_boards.c | 20 +++++++++++++++++++-
> >   1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c
> > index 3fb8478e72..79295a586c 100644
> > --- a/hw/arm/npcm8xx_boards.c
> > +++ b/hw/arm/npcm8xx_boards.c
> > @@ -27,6 +27,7 @@
> >   #include "qemu/error-report.h"
> >   #include "qemu/datadir.h"
> >   #include "qemu/units.h"
> > +#include "system/block-backend.h"
> >
> >   #define NPCM845_EVB_POWER_ON_STRAPS 0x000017ff
> >
> > @@ -59,10 +60,26 @@ static void npcm8xx_connect_flash(NPCM7xxFIUState *fiu, int cs_no,
> >   {
> >       DeviceState *flash;
> >       qemu_irq flash_cs;
> > +    BlockBackend *blk;
> > +    BlockDriverState *bs;
> > +    uint64_t blk_size, perm, shared_perm;
> >
> >       flash = qdev_new(flash_type);
> >       if (dinfo) {
> >           qdev_prop_set_drive(flash, "drive", blk_by_legacy_dinfo(dinfo));
> > +        blk = blk_by_legacy_dinfo(dinfo);
> > +        bs = blk_bs(blk);
> > +        blk_size = blk_getlength(blk);
> > +
> > +        if (!bdrv_is_read_only(bs)) {
>
> This isn't what I meant, we'll get the same issue with read-only storage.
>
> See:
> https://lore.kernel.org/qemu-devel/CAFEAcA9itEDTrznX1KRVEza__Dch95aBpPzbdTJ0-tuxwiHtoQ@mail.gmail.com/
>
> > +            if (blk_size < fiu->flash_size) {
> > +                blk_get_perm(blk, &perm, &shared_perm);
> > +                blk_set_perm(blk, BLK_PERM_ALL, BLK_PERM_ALL, &error_abort);
> > +                blk_truncate(blk, fiu->flash_size, true, PREALLOC_MODE_OFF,
> > +                             BDRV_REQ_ZERO_WRITE, &error_abort);
> > +                blk_set_perm(blk, perm, shared_perm, &error_abort);
> > +            }
> > +        }
> >       }
> >       qdev_realize_and_unref(flash, BUS(fiu->spi), &error_fatal);
> >
> > @@ -194,7 +211,8 @@ static void npcm845_evb_init(MachineState *machine)
> >       qdev_realize(DEVICE(soc), NULL, &error_fatal);
> >
> >       npcm8xx_load_bootrom(machine, soc);
> > -    npcm8xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0, 0));
> > +    npcm8xx_connect_flash(&soc->fiu[0], 0, "mx66l1g45g",
> > +                          drive_get(IF_MTD, 0, 0));
> >       npcm845_evb_i2c_init(soc);
> >       npcm845_evb_fan_init(NPCM8XX_MACHINE(machine), soc);
> >       npcm8xx_load_kernel(machine, soc);
>

Hi Philippe,
Thanks for your sharing. Now I understand why you say it won't work on
read-only storage.
Currently, without this change we cannot run the npcm845-evb of QEMU
with a large flash size (128 MB), including the latest OpenBMC build
image.
That's why we sync the method into npcm8xx_board for our npcm845 evb
board. It's seems not suitable for upstream, right? Thanks.

-- 
Best regards,
Tim Lee