Re: [PATCH V2] hw/riscv: virt: Remove size restriction for pflash

[Philippe Mathieu-Daudé]

On 7/11/22 14:06, Peter Maydell wrote:
> On Mon, 7 Nov 2022 at 13:03, Sunil V L <sunilvl@ventanamicro.com> wrote:
> > The pflash implementation currently assumes fixed size of the
> > backend storage. Due to this, the backend storage file needs to be
> > exactly of size 32M. Otherwise, there will be an error like below.
> >
> > "device requires 33554432 bytes, block backend provides 4194304 bytes"
> >
> > Fix this issue by using the actual size of the backing store.
> >
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > ---
>
> Do you really want the flash device size presented to the guest
> to be variable depending on what the user passed as a block backend?
> I don't think this is how we handle flash devices on other boards...

Ideally handling smaller/bigger backend size should be transparent for
machine frontend, but we never agreed on what are user expectations and
how to deal with such cases.

Long term I'd go for:

- if flash is read-only

  a/ bigger backend: display a warning and ignore extra backend data.

  b/ smaller backend: assume flash block is in erased state and fill
     missing gap with -1 (the default erase value), displaying a warning
     on startup.

- if flash is read-write

  a/ bigger backend: display a warning and ignore extra backend data.

  b/ smaller backend: add a property to pflash device to handle missing
     gap as erased data. If this flag is not set, display a hint and
     exit with an error.

In Sunil particular case, I suppose the issue comes from commit
334c388f25 ("hw/block/pflash_cfi0{1, 2}: Error out if device length
isn't a power of two") which I'm going to revert because the code
base is not ready for such check:

https://lore.kernel.org/qemu-devel/78b914c5-ce7e-1d4a-0a67-450f286eb869@linaro.org/

Regards,

Phil.