[U-Boot-Users] flash hardware protection with CFI driver does not autoprotect monitor completely

[Matthias Fuchs <matthias.fuchs@esd-electronics.com>]

Hi,

I noticed that the monitor flash autoprotection from drivers/cfi_flash.c is not completely safe.
It does not protect all bootloader sectors in some situations:

	/* Monitor protection ON by default */
#if (CFG_MONITOR_BASE >= CFG_FLASH_BASE)
	flash_protect (FLAG_PROTECT_SET,
		       CFG_MONITOR_BASE,
		       CFG_MONITOR_BASE + monitor_flash_len  - 1,
		       flash_get_info(CFG_MONITOR_BASE));
#endif

I noticed this on the APC405 board after modifying it to use the common CFI flash driver.
Here are some lines from its config file:

#define CFG_MONITOR_BASE	0xFFF80000
#define CFG_MONITOR_LEN		(512 * 1024)	/* Reserve 512 kB for Monitor	*/

#define CFG_FLASH_CFI         1       /* Flash is CFI conformant */
#define CFG_FLASH_CFI_DRIVER  1       /* Use the common driver */
#define CFG_MAX_FLASH_SECT    128     /* max num of sects on one chip */
#define CFG_MAX_FLASH_BANKS   2       /* max num of flash banks */
#define CFG_FLASH_PROTECTION  1       /* use hardware protection        */
#define CFG_FLASH_USE_BUFFER_WRITE 1  /* use buffered writes (20x faster)     */
#define CFG_FLASH_BANKS_LIST  {CFG_FLASH_BASE, CFG_FLASH_BASE + CFG_FLASH_INCREMENT}

With this setup 'CFG_MONITOR_BASE + monitor_flash_len  - 1' evalutaes to fffc00ff
and therefore the last sector will not be protected. But on 4xx CPUs you have the reset
vector in the last sector so you definetely want it to be protected.

=> flinfo
...
  FFDC0000        FFDE0000        FFE00000        FFE20000        FFE40000
  FFE60000        FFE80000        FFEA0000        FFEC0000        FFEE0000 E
  FFF00000 E      FFF20000 E      FFF40000 E      FFF60000 E      FFF80000   RO
  FFFA0000   RO   FFFC0000   RO   FFFE0000


Question: what's the best way to fix this? We could modify the call to flash_protect()
like this:

	flash_protect (FLAG_PROTECT_SET,
		       CFG_MONITOR_BASE,
		       CFG_MONITOR_BASE + CFG_MONITOR_LEN  - 1,
		       flash_get_info(CFG_MONITOR_BASE));

But I am not sure if this is fine for all architectures. Any ideas?

Matthias