Re: [PATCH] hw/block/pflash_cfi01: Always set romd mode when clearing wcycle and cmd

Oops, I forgot to cc Bin Guo on this.

-- PMM

On Tue, 18 Aug 2026 at 12:57, Peter Maydell <[email protected]> wrote:
>
> The pflash_cfi01 code has an invariant that the MemoryRegion is in
> ROMD mode if and only if wcycle == 0 && cmd == 0.  We rely on this
> for setting ROMD mode on an inbound migration.
>
> There is one corner case where the code clears wcycle and cmd without
> also setting ROMD mode on the MR: in the "should never happen" code
> path in pflash_read().  As the comment notes, that code really is
> unreachable (unless an inbound migration feeds us a bogus pfl->cmd
> value), so this isn't a problem in practice.  But it does make the
> code a little trickier to analyse.
>
> Pull out a function which does the "clear wcycle and cmd and set mode
> to ROMD", and use it in the three places that need to do this.  This
> makes it clearer that we are preserving our invariant, and tidies up
> the loose end noted in the commit message of 60d010f66f2ad7
> ("hw/block/pflash_cfi01: Restore ROMD mode after migration").
>
> Signed-off-by: Peter Maydell <[email protected]>
> ---
>  hw/block/pflash_cfi01.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index a13b91967e..486fc6350d 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -243,6 +243,18 @@ static uint32_t pflash_devid_query(PFlashCFI01 *pfl, hwaddr offset)
>      return resp;
>  }
>
> +static void pflash_set_to_read_array_mode(PFlashCFI01 *pfl)
> +{
> +    /*
> +     * Reset the flash device to its "just read the data" mode.
> +     * The command 0x00 is not assigned by the CFI open standard,
> +     * but QEMU historically uses it for the READ_ARRAY command (0xff).
> +     */
> +    pfl->wcycle = 0;
> +    pfl->cmd = 0x00;
> +    memory_region_rom_device_set_romd(&pfl->mem, true);
> +}
> +
>  static uint32_t pflash_data_read(PFlashCFI01 *pfl, hwaddr offset,
>                                   int width, int be)
>  {
> @@ -270,12 +282,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
>      default:
>          /* This should never happen : reset state & treat it as a read */
>          trace_pflash_read_unknown_state(pfl->name, pfl->cmd);
> -        pfl->wcycle = 0;
> -        /*
> -         * The command 0x00 is not assigned by the CFI open standard,
> -         * but QEMU historically uses it for the READ_ARRAY command (0xff).
> -         */
> -        pfl->cmd = 0x00;
> +        pflash_set_to_read_array_mode(pfl);
>          /* fall through to read code */
>      case 0x00: /* This model reset value for READ_ARRAY (not CFI compliant) */
>          /* Flash area read */
> @@ -652,9 +659,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>
>   mode_read_array:
>      trace_pflash_mode_read_array(pfl->name);
> -    memory_region_rom_device_set_romd(&pfl->mem, true);
> -    pfl->wcycle = 0;
> -    pfl->cmd = 0x00; /* This model reset value for READ_ARRAY (not CFI) */
> +    pflash_set_to_read_array_mode(pfl);
>  }
>
>
> @@ -873,13 +878,7 @@ static void pflash_cfi01_system_reset(DeviceState *dev)
>      PFlashCFI01 *pfl = PFLASH_CFI01(dev);
>
>      trace_pflash_reset(pfl->name);
> -    /*
> -     * The command 0x00 is not assigned by the CFI open standard,
> -     * but QEMU historically uses it for the READ_ARRAY command (0xff).
> -     */
> -    pfl->cmd = 0x00;
> -    pfl->wcycle = 0;
> -    memory_region_rom_device_set_romd(&pfl->mem, true);
> +    pflash_set_to_read_array_mode(pfl);
>      /*
>       * The WSM ready timer occurs at most 150ns after system reset.
>       * This model deliberately ignores this delay.