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

[Tim Lee <timlee660101@gmail.com>]

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