Re: [PATCH] hw/block/pflash_cfi01: Limit maximum flash size to 256 MiB

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 4 Jun 2020 at 16:16, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 5/25/20 10:59 PM, Peter Maydell wrote:
> > What problem is this check solving? Is there some implementation
> > imposed limitation or a limit within the flash header format
> > that means larger sizes don't work? If so, what's the restriction?
>
> I am not confident maintaining virtual device with no specifications.

This isn't a virtual device, is it? The CFI spec exists and defines
what these flash devices behave like.

> If someone come with a datasheet for a pflash > 256MB then we can add
> another commit to relax this restriction.
> If someone is forced to use a >256MB and such hardware does not exist,
> I'd rather have a document in docs/specs/pflash_cfi01.rst describing the
> CFI fields.
>
> IOW this is not a hardware limitation, but a maintenance protection.
>
> I am worried with the recent EDK2 activity with the SBSA-ref, and I
> don't want to give false hope to developers that they can create any
> kind of pflash with the current device model.
>
> If I reword this better in the commit description, is my argument
> acceptable?

Not really; I think we should know what we're limiting against.
Currently you're checking total_len, but this is just sector_len * nb_blocs,
so if there's a problem with silly large values then it's probably
actually a problem with one of those being over-sized which would
still show up even if the total_len was less than 256MB.
(I suspect the underlying limit here is what the cfi_table entries
0x2D..0x30 impose on blocks_per_device and sector_len_per_device.)

thanks
-- PMM