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

Matthias Fuchs
2007-11-18T21:14:05Z

Hi Wolfgang,

On Sunday 18 November 2007 20:31, Wolfgang Denk wrote:
> In message <200711181959.55060.matthias.fuchs@esd-electronics.com> you wrote:
> >
> > 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:
> ...
> > Question: what's the best way to fix this? We could modify the call to flash_protect()
>
> The bst way is to make sure that all sectors that need to be protected
> do get protected.
sure:-)
>
> > 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?
>
> The problem is that you don't have any guarantee that CFG_MONITOR_LEN
> includes the reset vector; also, there might be configurations where
> the U-Boot image is not stored at the end of the flash, so there is a
> bigger gap between the image and the sector with the reset vector,
> and it would be not good to enforce protection on that area that
> might be useful to the user otherwise.
>
> I think as a short term fix we might define an additional area that
> needs to be protected (the reset vector). Mid/long term we should
> change the code so you can pass a list of areas (start/end or
> start/length pairs) that will be protected. This would, for example,
> also allow to keep certain other areas (FDT, kernel image, etc.)
> auto-protected as well - configurable by the user ona per-board base.

Sometinhg like this?

Add this to board's config file:
#define CONFIG_FLASH_AUTOPROTECT_LIST {{0xfffe0000, 0xffffffff}, {0xfe000000, 0xfe0fffff}}

And handle this list from the cfi driver:

diff --git a/drivers/cfi_flash.c b/drivers/cfi_flash.c
index 5579a1e..0777349 100644
--- a/drivers/cfi_flash.c
+++ b/drivers/cfi_flash.c
@@ -333,11 +333,17 @@ ulong flash_read_long (flash_info_t * info, flash_sect_t sect, uint offset)

/*-----------------------------------------------------------------------
*/
+struct apl_s {ulong start; ulong end;};
+
unsigned long flash_init (void)
{
unsigned long size = 0;
int i;

+#if defined(CONFIG_FLASH_AUTOPROTECT_LIST)
+ struct apl_s apl[] = CONFIG_FLASH_AUTOPROTECT_LIST;
+#endif
+
#ifdef CFG_FLASH_PROTECTION
char *s = getenv("unlock");
#endif
@@ -419,6 +425,16 @@ unsigned long flash_init (void)
CFG_ENV_ADDR_REDUND + CFG_ENV_SIZE_REDUND - 1,
flash_get_info(CFG_ENV_ADDR_REDUND));
#endif
+
+#if defined(CONFIG_FLASH_AUTOPROTECT_LIST)
+ for (i = 0; i < sizeof(apl)/sizeof(struct apl_s); i++) {
+ printf("autoprotecting from %08x to %08x\n", apl[i].start, apl[i].end);
+ flash_protect (FLAG_PROTECT_SET,
+ apl[i].start,
+ apl[i].end,
+ flash_get_info(apl[i].start));
+ }
+#endif
return (size);
}

Good night.

Matthias