[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/block/pflash_cfi01: Limit maximum flash size to 256 MiB
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH] hw/block/pflash_cfi01: Limit maximum flash size to 256 MiB |
Date: |
Thu, 4 Jun 2020 17:55:51 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 6/4/20 5:30 PM, Peter Maydell wrote:
> 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.)
What I'm working on is a whitelist of the few models our machines really
use, but it is taking time. Meanwhile I wanted to at least limit the
total size.
I'll try to finish my whitelist effort before someone try to use a
>256MB flash.
>
> thanks
> -- PMM
>