[U-Boot] SPI flash protection

[Albert ARIBAUD <albert.aribaud@free.fr>]

Le 31/01/2011 01:31, Simon Guinot a ?crit :

> On Sun, Jan 30, 2011 at 06:19:20PM +0100, Albert ARIBAUD wrote:
>> Le 30/01/2011 15:56, Simon Guinot a ?crit :
>>
>>>> So, if SPI Flash protection is possible on block or sector levels,
>>>> protect the blocks/sectors where U-Boot is located, including the
>>>> environment.
>>>
>>> For a Macronix SPI flash, the status register export 3 bits (or 4
>>> depending the model and size) to configure the block protection.
>>>
>>> Here is the protect area map for a MX25L4005A 4Mb flash:
>>>
>>>    bit  2 1 0 | protect level
>>>   ____________|_______________
>>>         0 0 0 | none
>>>         0 0 1 | block 7
>>>         0 1 0 | block 6-7
>>>         0 1 1 | block 4-7
>>>         1 * * | all
>>>
>>> As you can figure, deal with a per block protection in a generic way is
>>> not easy..
>>
>> We don't necessarily have a generic per-block protection scheme at
>> the HW level, but we can still use one as an API leve, i.e. have
>> protect and unprotect functions which take generic start and end
>> addresses and do what they can with the HW underneath (IIRC, protect
>> on and protect off commands can use any address, and will check with
>> the HW).
>
> Do you suggest to add a protect function to the SPI flash subsystem ?

If it is not there already, yes: analogous to what the NOR Flash 
provides, so that from the U-Boot command line, erasing U-Boot on the 
SPI Flash would require un-protecting it first.

>> In your case, the HW seems to allow protecting 1, 2, 4 or all blocks
>> from top of SPI Flash (I assume the boot process fetches code from a
>> protectable areas, inside block 7 I imagine). I also assume you put
>> U-Boot in the top blocks -- 7 alone, or 7 and 6 if too big for one
>> block, etc.
>
> Block size is 64KB. The compiled U-Boot image is about 220KB and
> environment is 4KB. So, 4 blocks are required for U-Boot.
>
>> Can the HW erase only one sector inside a block? If not, then you'd
>> have to sacrifice a whole block for the env. Otherwise, you can lump
>> together a sector for env and the sectors for U-Boot, hopefully in a
>> single block.
>
>
> Sector size is 4KB. Both blocks and sectors can be erased/written
> individually.
>
> Looking at macronix_erase(), I can see:
>
>          /*
>           * This function currently uses sector erase only.
>           * probably speed things up by using bulk erase
>           * when possible.
>           */
>
>          sector_size =3D mcx->params->page_size * mcx->params->pages_per_sector
>                          * mcx->params->sectors_per_block;
>
>          if (offset % sector_size || len % sector_size) {
>                  debug("SF: Erase offset/length not multiple of sector size\n");
>                  return -1;
>          }
>
> The comments and the variable name "sector_size" are misleading. The
> effective granularity is block size: 64KB. Maybe a bug...

Dunno. Still, the function takes a generic offset/length, and could be 
passed these from the U-Boot command line.

>>>> Saveenv should unprotect and re-protect the environment sector/block.
>>>
>>> If I understand well, macronix_{erase,write}() are called from either
>>> "saveenv" or "spi erase/write" commands and the low level macronix
>>> functions don't have any flash map knowledge. Then, leave protect or not
>>> a flash sector after a write/erase operation is not an easy decision.
>>>
>>> Maybe the status register protection can be configured during the board
>>> initialization ? after all, the required flash informations (as device
>>> size, u-boot and environment localization) are available within the
>>> board setup code.
>>
>> Can't the status register be re-programmed? Is it a kind of "protect
>> now and until power-off" mechanism? If it is, then rewriting the
>> environment will not be possible with this protection mechanism, and
>> you might have to fake protection by software inside the driver if
>> you need it. :(
>
> The status register can be re-programmed. The "block protect" bits and
> the "write enable" bit are persistent (not reseted after a power-off).
> I am not sure I understand well what do you mean by "fake protection
> by software inside the driver"...

Never mind what I meant -- since the status register can be 
reprogrammed, you can do a protect off then protect on without problems. :)

> Simon

Amicalement,
-- 
Albert.