Re: Supporting flash that is locked by default

Todd Poynor
2004-09-29T02:01:12Z

Nicolas Pitre wrote:
> On Tue, 28 Sep 2004, Todd Poynor wrote:
>>Here's a try at automatically unlocking all blocks of Intel CFI flash at
>>probe time and at system resume time. The new logic is in the chip
>>driver, but perhaps it would be better to move it generic, adding a new
>>chip driver callback for "is unlock needed?"; can rework things that way
>>if desired (was originally hoping to hide this "feature" from generic
>>code).
>>
>>I did not notice a query feature bit that identifies whether the flash
>>has the "all blocks locked at power on" property in the datasheets, so
>>the patch checks whether block 0 is locked and assumes its one of those
>>flashes if so.
>
>
> This is a rather bad idea since block 0 is often the bootloader and most
> likely to be locked on purpose.
>
> I'm a bit unconfortable with a feature like this as well. What if I
> want some partitions to remain locked at all times at the hw level?

The intention was to catch the power-on behavior and normalize it to
"all unlocked" like traditional flashes, then allow system-specific
actions (or map partition software locking) to lock blocks, as is
commonly done on a Rev. J flash, for example. But it may be the case
that some folks like the default-locked CFI behavior of these flashes
and don't want the "all unlocked by default" common denominator, so
agreed this should be rethought.

> I think the decision of unlocking the chip and what to unlock should
> remain in the map driver since this is where the knowledge of what the
> flash is meant to be used for is the most likely to be found.

Or require all the policy on this matter to reside in userspace,
requiring an explicit flash_unlock for partitions, and not have the
kernel automatically unlock anything. If the map partition writeable
flags are proof enough the partition should be automatically unlocked
then sounds good to me, I'll send a patch.

>>Since all blocks may be unlocked at resume time, it would be up to a
>>userspace action to relock any previously explicitly locked blocks soon
>>afterwards.
>
>
> This sounds really really fishy to me.
>
> If you can query which blocks are or are not locked before suspending,
> then why don't you just create a bitmap with the state of the entire
> flash and restore it upon resume?

I was planning what you describe for "phase 2" (the userspace actions to
re-lock are actually the converse of how it works today, where actions
are required to unlock), thought maybe adding this would be
controversial. I'll send a patch to do this. Thanks,

--
Todd