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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.