diff mbox

[U-Boot] Flash protection and fw_setenv tool

Message ID E1C9442006A9AE48A7A4B85E0D01C4D45B47FD@stwexdb.stww2k.local
State Not Applicable
Headers show

Commit Message

Waibel Georg Feb. 26, 2013, 12:45 p.m. UTC 
Hi again,

the previous patch missed some places to apply the unprotect switch. Here's an improved version:

Comments

Dirk Eibach Feb. 26, 2013, 2:06 p.m. UTC | #1 
Hi Georg,

maybe removing CONFIG_SYS_FLASH_PROTECTION from your configuration does already what you want to achieve.

BTW Linux support should be available here:
http://patchwork.ozlabs.org/patch/213602/

Cheers
Dirk

> -----Original Message-----
> From: u-boot-bounces@lists.denx.de 
> [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Waibel Georg
> Sent: Tuesday, February 26, 2013 1:46 PM
> To: u-boot@lists.denx.de
> Subject: Re: [U-Boot] Flash protection and fw_setenv tool
> 
> Hi again,
> 
> the previous patch missed some places to apply the unprotect 
> switch. Here's an improved version:
> 
> Index: common/env_flash.c
> ===================================================================
> --- common/env_flash.c	(revision 4065)
> +++ common/env_flash.c	(working copy)
> @@ -218,9 +218,12 @@
>  done:
>  	if (saved_data)
>  		free(saved_data);
> +
> +#ifndef CONFIG_ENV_UNPROTECTED
>  	/* try to re-protect */
>  	flash_sect_protect(1, (ulong)flash_addr, end_addr);
>  	flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
> +#endif
>  
>  	return rc;
>  }
> @@ -310,7 +313,9 @@
>  	if (saved_data)
>  		free(saved_data);
>  	/* try to re-protect */
> +#ifndef CONFIG_ENV_UNPROTECTED
>  	flash_sect_protect(1, (long)flash_addr, end_addr);
> +#endif
>  	return rc;
>  }
>  #endif /* CMD_SAVEENV */
> @@ -340,7 +345,9 @@
>  		flash_write(&flag,
>  			    (ulong)&(flash_addr_new->flags),
>  			    sizeof(flash_addr_new->flags));
> +#ifndef CONFIG_ENV_UNPROTECTED
>  		flash_sect_protect(1, (ulong)flash_addr_new, 
> end_addr_new);
> +#endif
>  	}
>  
>  	if (flash_addr->flags != ACTIVE_FLAG && @@ -352,7 +359,9 @@
>  		flash_write(&flag,
>  			    (ulong)&(flash_addr->flags),
>  			    sizeof(flash_addr->flags));
> +#ifndef CONFIG_ENV_UNPROTECTED
>  		flash_sect_protect(1, (ulong)flash_addr, end_addr);
> +#endif
>  	}
>  
>  	if (gd->env_valid == 2)
> Index: drivers/mtd/cfi_flash.c
> ===================================================================
> --- drivers/mtd/cfi_flash.c	(revision 4065)
> +++ drivers/mtd/cfi_flash.c	(working copy)
> @@ -2294,7 +2294,7 @@
>  #endif
>  
>  	/* Environment protection ON by default */ -#ifdef 
> CONFIG_ENV_IS_IN_FLASH
> +#if defined(CONFIG_ENV_IS_IN_FLASH) && 
> !defined(CONFIG_ENV_UNPROTECTED)
>  	flash_protect(FLAG_PROTECT_SET,
>  		       CONFIG_ENV_ADDR,
>  		       CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 
> 1, @@ -2302,7 +2302,7 @@  #endif
>  
>  	/* Redundant environment protection ON by default */ 
> -#ifdef CONFIG_ENV_ADDR_REDUND
> +#if defined(CONFIG_ENV_ADDR_REDUND) && 
> !defined(CONFIG_ENV_UNPROTECTED)
>  	flash_protect(FLAG_PROTECT_SET,
>  		       CONFIG_ENV_ADDR_REDUND,
>  		       CONFIG_ENV_ADDR_REDUND + 
> CONFIG_ENV_SECT_SIZE - 1,
> 
> Regards
> Georg
> 
> 
> 
> ------------------------------------------------------------------------------------------------
Messe-Highlights 2013. Wir freuen uns auf Ihren Besuch.

Broadcast Video Expo 2013  
London - 26.02. bis 28.02.2013 - Stand B22                                                

CeBIT 2013 
In Hannover - 05.03. bis 09.03.2012 - Halle 11, Stand D31

ATC Global 2013 
Amsterdam - 12.03. bis 14.03.2012 - Halle 10, Stand D202
------------------------------------------------------------------------------------------------
Besuchen Sie unseren Blog auf
http://blog.gdsys.de

oder folgen Sie uns auf:
twitter: http://twitter.com/#!/gdsys
facebook: http://www.facebook.com/pages/Guntermann-Drunck-GmbH/318396891518396
Google+ : https://plus.google.com/100228872787564309232/ 
YouTube: http://www.youtube.com/user/GuntermannDrunck
------------------------------------------------------------------------------------------------
Guntermann & Drunck GmbH Systementwicklung 
Dortmunder Str. 4a 
D-57234 Wilnsdorf - Germany 
Tel: +49 (0) 27 39 / 89 01 - 100  Fax: +49 (0) 27 39 / 89 01 - 120 
E-Mail: sales@gdsys.de - Web: www.gdsys.de
------------------------------------------------------------------------------------------------
Geschäftsführer: 
Udo Guntermann - Martin Drunck - Reiner Ruelmann
HRB 2884, Amtsgericht Siegen - WEEE-Reg.-Nr. DE30763240
USt.-Id.-Nr. DE 126575222 - Steuer-Nr. 342 / 5835 / 1041
------------------------------------------------------------------------------------------------
DQS-zertifiziert nach ISO 9001:2008
------------------------------------------------------------------------------------------------

-----Ursprüngliche Nachricht-----
> Von: u-boot-bounces@lists.denx.de 
> [mailto:u-boot-bounces@lists.denx.de] Im Auftrag von Waibel Georg
> Gesendet: Dienstag, 26. Februar 2013 12:05
> An: 'u-boot@lists.denx.de'
> Betreff: [U-Boot] Flash protection and fw_setenv tool
> 
> Hi,
> 
> I activated flash sector protection for our CFI NOR flash 
> chip (CONFIG_SYS_FLASH_PROTECTION). Protection bits for 
> U-Boot code sectors and environment sector are set by default 
> during U-Boot startup. Thus from Linux, I cannot write to 
> this sectors, which is generally a correct behavior. BUT, 
> with this, changing the environment from user space with the 
> fw_setenv tool does not work anymore. 
> And here's the question: How to deal with this issue 
> correctly? Two approaches came in my mind:
> a)
> Unprotecting the flash sectors by the Linux kernel (in CFI 
> driver). This seems not to be implemented yet, at least for 
> AMD CFI command set (0x0002). 
> However, when unprotecting flash sectors from Linux side, 
> does a sector protection actually makes sense?
> b)
> Introduce a config option in U-Boot which allows to leave the 
> environment unprotected. See patch below. I called this 
> option CONFIG_ENV_UNPROTECTED When defined, U-Boot does not 
> protect the environment. For compatibility with boards in 
> which the environment sector is already protected, I left the 
> unprotect part in the saveenv function untouched.
> 
> Any comments about this issue?
> Thanks and regards
> Georg
> 
> Index: common/env_flash.c
> ===================================================================
> --- common/env_flash.c	(revision 3966)
> +++ common/env_flash.c	(working copy)
> @@ -218,9 +218,12 @@
>  done:
>  	if (saved_data)
>  		free(saved_data);
> +
> +#ifndef CONFIG_ENV_UNPROTECTED
>  	/* try to re-protect */
>  	flash_sect_protect(1, (ulong)flash_addr, end_addr);
>  	flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
> +#endif
>  
>  	return rc;
>  }
> Index: drivers/mtd/cfi_flash.c
> ===================================================================
> --- drivers/mtd/cfi_flash.c	(revision 3966)
> +++ drivers/mtd/cfi_flash.c	(working copy)
> @@ -2294,7 +2294,7 @@
>  #endif
>  
>  	/* Environment protection ON by default */ -#ifdef 
> CONFIG_ENV_IS_IN_FLASH
> +#if defined(CONFIG_ENV_IS_IN_FLASH) && 
> !defined(CONFIG_ENV_UNPROTECTED)
>  	flash_protect(FLAG_PROTECT_SET,
>  		       CONFIG_ENV_ADDR,
>  		       CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1,
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 
> 
>
Waibel Georg Feb. 26, 2013, 3:04 p.m. UTC | #2 
Hi Dirk,
I enabled the flash protection to protect the UBoot from being overwritten, since this means that customers need to send back the device for reprogramming the UBoot (without a valid bootloader, our MPC5200 target can only be accesses by a programmer like the BDI2000 debugger). Thus for our application it makes sense to have the protection on. Having the CONFIG_ENV_UNPROTECTED option fits to our requirements pretty well.
I was just wondering if this option is something that is commonly useful...
Thanks and regards
Georg



> -----Ursprüngliche Nachricht-----
> Von: Eibach, Dirk [mailto:Eibach@gdsys.de]
> Gesendet: Dienstag, 26. Februar 2013 15:07
> An: Waibel Georg; u-boot@lists.denx.de
> Cc: Stefan Roese
> Betreff: RE: [U-Boot] Flash protection and fw_setenv tool
> 
> Hi Georg,
> 
> maybe removing CONFIG_SYS_FLASH_PROTECTION from your configuration
> does already what you want to achieve.
> 
> BTW Linux support should be available here:
> http://patchwork.ozlabs.org/patch/213602/
> 
> Cheers
> Dirk
> 
> > -----Original Message-----
> > From: u-boot-bounces@lists.denx.de
> > [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Waibel Georg
> > Sent: Tuesday, February 26, 2013 1:46 PM
> > To: u-boot@lists.denx.de
> > Subject: Re: [U-Boot] Flash protection and fw_setenv tool
> >
> > Hi again,
> >
> > the previous patch missed some places to apply the unprotect switch.
> > Here's an improved version:
> >
> > Index: common/env_flash.c
> >
> ==========================================================
> =========
> > --- common/env_flash.c	(revision 4065)
> > +++ common/env_flash.c	(working copy)
> > @@ -218,9 +218,12 @@
> >  done:
> >  	if (saved_data)
> >  		free(saved_data);
> > +
> > +#ifndef CONFIG_ENV_UNPROTECTED
> >  	/* try to re-protect */
> >  	flash_sect_protect(1, (ulong)flash_addr, end_addr);
> >  	flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
> > +#endif
> >
> >  	return rc;
> >  }
> > @@ -310,7 +313,9 @@
> >  	if (saved_data)
> >  		free(saved_data);
> >  	/* try to re-protect */
> > +#ifndef CONFIG_ENV_UNPROTECTED
> >  	flash_sect_protect(1, (long)flash_addr, end_addr);
> > +#endif
> >  	return rc;
> >  }
> >  #endif /* CMD_SAVEENV */
> > @@ -340,7 +345,9 @@
> >  		flash_write(&flag,
> >  			    (ulong)&(flash_addr_new->flags),
> >  			    sizeof(flash_addr_new->flags));
> > +#ifndef CONFIG_ENV_UNPROTECTED
> >  		flash_sect_protect(1, (ulong)flash_addr_new,
> end_addr_new);
> > +#endif
> >  	}
> >
> >  	if (flash_addr->flags != ACTIVE_FLAG && @@ -352,7 +359,9 @@
> >  		flash_write(&flag,
> >  			    (ulong)&(flash_addr->flags),
> >  			    sizeof(flash_addr->flags));
> > +#ifndef CONFIG_ENV_UNPROTECTED
> >  		flash_sect_protect(1, (ulong)flash_addr, end_addr);
> > +#endif
> >  	}
> >
> >  	if (gd->env_valid == 2)
> > Index: drivers/mtd/cfi_flash.c
> >
> ==========================================================
> =========
> > --- drivers/mtd/cfi_flash.c	(revision 4065)
> > +++ drivers/mtd/cfi_flash.c	(working copy)
> > @@ -2294,7 +2294,7 @@
> >  #endif
> >
> >  	/* Environment protection ON by default */ -#ifdef
> > CONFIG_ENV_IS_IN_FLASH
> > +#if defined(CONFIG_ENV_IS_IN_FLASH) &&
> > !defined(CONFIG_ENV_UNPROTECTED)
> >  	flash_protect(FLAG_PROTECT_SET,
> >  		       CONFIG_ENV_ADDR,
> >  		       CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1, @@ -
> 2302,7
> > +2302,7 @@  #endif
> >
> >  	/* Redundant environment protection ON by default */ -#ifdef
> > CONFIG_ENV_ADDR_REDUND
> > +#if defined(CONFIG_ENV_ADDR_REDUND) &&
> > !defined(CONFIG_ENV_UNPROTECTED)
> >  	flash_protect(FLAG_PROTECT_SET,
> >  		       CONFIG_ENV_ADDR_REDUND,
> >  		       CONFIG_ENV_ADDR_REDUND +
> > CONFIG_ENV_SECT_SIZE - 1,
> >
> > Regards
> > Georg
> >
> >
> >
> > ----------------------------------------------------------------------
> > --------------------------
> Messe-Highlights 2013. Wir freuen uns auf Ihren Besuch.
> 
> Broadcast Video Expo 2013
> London - 26.02. bis 28.02.2013 - Stand B22
> 
> CeBIT 2013
> In Hannover - 05.03. bis 09.03.2012 - Halle 11, Stand D31
> 
> ATC Global 2013
> Amsterdam - 12.03. bis 14.03.2012 - Halle 10, Stand D202
> ----------------------------------------------------------------------------------------------
> --
> Besuchen Sie unseren Blog auf
> http://blog.gdsys.de
> 
> oder folgen Sie uns auf:
> twitter: http://twitter.com/#!/gdsys
> facebook: http://www.facebook.com/pages/Guntermann-Drunck-
> GmbH/318396891518396
> Google+ : https://plus.google.com/100228872787564309232/
> YouTube: http://www.youtube.com/user/GuntermannDrunck
> ----------------------------------------------------------------------------------------------
> --
> Guntermann & Drunck GmbH Systementwicklung Dortmunder Str. 4a
> D-57234 Wilnsdorf - Germany
> Tel: +49 (0) 27 39 / 89 01 - 100  Fax: +49 (0) 27 39 / 89 01 - 120
> E-Mail: sales@gdsys.de - Web: www.gdsys.de
> ----------------------------------------------------------------------------------------------
> --
> Geschäftsführer:
> Udo Guntermann - Martin Drunck - Reiner Ruelmann HRB 2884, Amtsgericht
> Siegen - WEEE-Reg.-Nr. DE30763240 USt.-Id.-Nr. DE 126575222 - Steuer-Nr.
> 342 / 5835 / 1041
> ----------------------------------------------------------------------------------------------
> --
> DQS-zertifiziert nach ISO 9001:2008
> ----------------------------------------------------------------------------------------------
> --
> 
> -----Ursprüngliche Nachricht-----
> > Von: u-boot-bounces@lists.denx.de
> > [mailto:u-boot-bounces@lists.denx.de] Im Auftrag von Waibel Georg
> > Gesendet: Dienstag, 26. Februar 2013 12:05
> > An: 'u-boot@lists.denx.de'
> > Betreff: [U-Boot] Flash protection and fw_setenv tool
> >
> > Hi,
> >
> > I activated flash sector protection for our CFI NOR flash chip
> > (CONFIG_SYS_FLASH_PROTECTION). Protection bits for U-Boot code
> sectors
> > and environment sector are set by default during U-Boot startup. Thus
> > from Linux, I cannot write to this sectors, which is generally a
> > correct behavior. BUT, with this, changing the environment from user
> > space with the fw_setenv tool does not work anymore.
> > And here's the question: How to deal with this issue correctly? Two
> > approaches came in my mind:
> > a)
> > Unprotecting the flash sectors by the Linux kernel (in CFI driver).
> > This seems not to be implemented yet, at least for AMD CFI command set
> > (0x0002).
> > However, when unprotecting flash sectors from Linux side, does a
> > sector protection actually makes sense?
> > b)
> > Introduce a config option in U-Boot which allows to leave the
> > environment unprotected. See patch below. I called this option
> > CONFIG_ENV_UNPROTECTED When defined, U-Boot does not protect the
> > environment. For compatibility with boards in which the environment
> > sector is already protected, I left the unprotect part in the saveenv
> > function untouched.
> >
> > Any comments about this issue?
> > Thanks and regards
> > Georg
> >
> > Index: common/env_flash.c
> >
> ==========================================================
> =========
> > --- common/env_flash.c	(revision 3966)
> > +++ common/env_flash.c	(working copy)
> > @@ -218,9 +218,12 @@
> >  done:
> >  	if (saved_data)
> >  		free(saved_data);
> > +
> > +#ifndef CONFIG_ENV_UNPROTECTED
> >  	/* try to re-protect */
> >  	flash_sect_protect(1, (ulong)flash_addr, end_addr);
> >  	flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
> > +#endif
> >
> >  	return rc;
> >  }
> > Index: drivers/mtd/cfi_flash.c
> >
> ==========================================================
> =========
> > --- drivers/mtd/cfi_flash.c	(revision 3966)
> > +++ drivers/mtd/cfi_flash.c	(working copy)
> > @@ -2294,7 +2294,7 @@
> >  #endif
> >
> >  	/* Environment protection ON by default */ -#ifdef
> > CONFIG_ENV_IS_IN_FLASH
> > +#if defined(CONFIG_ENV_IS_IN_FLASH) &&
> > !defined(CONFIG_ENV_UNPROTECTED)
> >  	flash_protect(FLAG_PROTECT_SET,
> >  		       CONFIG_ENV_ADDR,
> >  		       CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1,
> >
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
> >
> >
> 
>
Waibel Georg Feb. 27, 2013, 10:32 a.m. UTC | #3 
Hi again,
I just found this thread: [U-Boot] fw_setenv on protected flash
It deals with exactly the same problem. I updated my kernel with the patch mentioned there to support flash protection and everything is ok now.
The patch I provided here is useless now. 
Regards
Georg

> -----Ursprüngliche Nachricht-----
> Von: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de]
> Im Auftrag von Waibel Georg
> Gesendet: Dienstag, 26. Februar 2013 16:04
> An: 'Eibach, Dirk'; u-boot@lists.denx.de
> Cc: Stefan Roese
> Betreff: Re: [U-Boot] Flash protection and fw_setenv tool
> 
> Hi Dirk,
> I enabled the flash protection to protect the UBoot from being overwritten,
> since this means that customers need to send back the device for
> reprogramming the UBoot (without a valid bootloader, our MPC5200 target
> can only be accesses by a programmer like the BDI2000 debugger). Thus for
> our application it makes sense to have the protection on. Having the
> CONFIG_ENV_UNPROTECTED option fits to our requirements pretty well.
> I was just wondering if this option is something that is commonly useful...
> Thanks and regards
> Georg
> 
> 
> 
> > -----Ursprüngliche Nachricht-----
> > Von: Eibach, Dirk [mailto:Eibach@gdsys.de]
> > Gesendet: Dienstag, 26. Februar 2013 15:07
> > An: Waibel Georg; u-boot@lists.denx.de
> > Cc: Stefan Roese
> > Betreff: RE: [U-Boot] Flash protection and fw_setenv tool
> >
> > Hi Georg,
> >
> > maybe removing CONFIG_SYS_FLASH_PROTECTION from your
> configuration
> > does already what you want to achieve.
> >
> > BTW Linux support should be available here:
> > http://patchwork.ozlabs.org/patch/213602/
> >
> > Cheers
> > Dirk
> >
> > > -----Original Message-----
> > > From: u-boot-bounces@lists.denx.de
> > > [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Waibel Georg
> > > Sent: Tuesday, February 26, 2013 1:46 PM
> > > To: u-boot@lists.denx.de
> > > Subject: Re: [U-Boot] Flash protection and fw_setenv tool
> > >
> > > Hi again,
> > >
> > > the previous patch missed some places to apply the unprotect switch.
> > > Here's an improved version:
> > >
> > > Index: common/env_flash.c
> > >
> >
> ==========================================================
> > =========
> > > --- common/env_flash.c	(revision 4065)
> > > +++ common/env_flash.c	(working copy)
> > > @@ -218,9 +218,12 @@
> > >  done:
> > >  	if (saved_data)
> > >  		free(saved_data);
> > > +
> > > +#ifndef CONFIG_ENV_UNPROTECTED
> > >  	/* try to re-protect */
> > >  	flash_sect_protect(1, (ulong)flash_addr, end_addr);
> > >  	flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
> > > +#endif
> > >
> > >  	return rc;
> > >  }
> > > @@ -310,7 +313,9 @@
> > >  	if (saved_data)
> > >  		free(saved_data);
> > >  	/* try to re-protect */
> > > +#ifndef CONFIG_ENV_UNPROTECTED
> > >  	flash_sect_protect(1, (long)flash_addr, end_addr);
> > > +#endif
> > >  	return rc;
> > >  }
> > >  #endif /* CMD_SAVEENV */
> > > @@ -340,7 +345,9 @@
> > >  		flash_write(&flag,
> > >  			    (ulong)&(flash_addr_new->flags),
> > >  			    sizeof(flash_addr_new->flags));
> > > +#ifndef CONFIG_ENV_UNPROTECTED
> > >  		flash_sect_protect(1, (ulong)flash_addr_new,
> > end_addr_new);
> > > +#endif
> > >  	}
> > >
> > >  	if (flash_addr->flags != ACTIVE_FLAG && @@ -352,7 +359,9 @@
> > >  		flash_write(&flag,
> > >  			    (ulong)&(flash_addr->flags),
> > >  			    sizeof(flash_addr->flags));
> > > +#ifndef CONFIG_ENV_UNPROTECTED
> > >  		flash_sect_protect(1, (ulong)flash_addr, end_addr);
> > > +#endif
> > >  	}
> > >
> > >  	if (gd->env_valid == 2)
> > > Index: drivers/mtd/cfi_flash.c
> > >
> >
> ==========================================================
> > =========
> > > --- drivers/mtd/cfi_flash.c	(revision 4065)
> > > +++ drivers/mtd/cfi_flash.c	(working copy)
> > > @@ -2294,7 +2294,7 @@
> > >  #endif
> > >
> > >  	/* Environment protection ON by default */ -#ifdef
> > > CONFIG_ENV_IS_IN_FLASH
> > > +#if defined(CONFIG_ENV_IS_IN_FLASH) &&
> > > !defined(CONFIG_ENV_UNPROTECTED)
> > >  	flash_protect(FLAG_PROTECT_SET,
> > >  		       CONFIG_ENV_ADDR,
> > >  		       CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1, @@ -
> > 2302,7
> > > +2302,7 @@  #endif
> > >
> > >  	/* Redundant environment protection ON by default */ -#ifdef
> > > CONFIG_ENV_ADDR_REDUND
> > > +#if defined(CONFIG_ENV_ADDR_REDUND) &&
> > > !defined(CONFIG_ENV_UNPROTECTED)
> > >  	flash_protect(FLAG_PROTECT_SET,
> > >  		       CONFIG_ENV_ADDR_REDUND,
> > >  		       CONFIG_ENV_ADDR_REDUND +
> > > CONFIG_ENV_SECT_SIZE - 1,
> > >
> > > Regards
> > > Georg
> > >
> > >
> > >
> > > --------------------------------------------------------------------
> > > --
> > > --------------------------
> > Messe-Highlights 2013. Wir freuen uns auf Ihren Besuch.
> >
> > Broadcast Video Expo 2013
> > London - 26.02. bis 28.02.2013 - Stand B22
> >
> > CeBIT 2013
> > In Hannover - 05.03. bis 09.03.2012 - Halle 11, Stand D31
> >
> > ATC Global 2013
> > Amsterdam - 12.03. bis 14.03.2012 - Halle 10, Stand D202
> > ----------------------------------------------------------------------
> > ------------------------
> > --
> > Besuchen Sie unseren Blog auf
> > http://blog.gdsys.de
> >
> > oder folgen Sie uns auf:
> > twitter: http://twitter.com/#!/gdsys
> > facebook: http://www.facebook.com/pages/Guntermann-Drunck-
> > GmbH/318396891518396
> > Google+ : https://plus.google.com/100228872787564309232/
> > YouTube: http://www.youtube.com/user/GuntermannDrunck
> > ----------------------------------------------------------------------
> > ------------------------
> > --
> > Guntermann & Drunck GmbH Systementwicklung Dortmunder Str. 4a
> > D-57234 Wilnsdorf - Germany
> > Tel: +49 (0) 27 39 / 89 01 - 100  Fax: +49 (0) 27 39 / 89 01 - 120
> > E-Mail: sales@gdsys.de - Web: www.gdsys.de
> > ----------------------------------------------------------------------
> > ------------------------
> > --
> > Geschäftsführer:
> > Udo Guntermann - Martin Drunck - Reiner Ruelmann HRB 2884,
> Amtsgericht
> > Siegen - WEEE-Reg.-Nr. DE30763240 USt.-Id.-Nr. DE 126575222 - Steuer-Nr.
> > 342 / 5835 / 1041
> > ----------------------------------------------------------------------
> > ------------------------
> > --
> > DQS-zertifiziert nach ISO 9001:2008
> > ----------------------------------------------------------------------
> > ------------------------
> > --
> >
> > -----Ursprüngliche Nachricht-----
> > > Von: u-boot-bounces@lists.denx.de
> > > [mailto:u-boot-bounces@lists.denx.de] Im Auftrag von Waibel Georg
> > > Gesendet: Dienstag, 26. Februar 2013 12:05
> > > An: 'u-boot@lists.denx.de'
> > > Betreff: [U-Boot] Flash protection and fw_setenv tool
> > >
> > > Hi,
> > >
> > > I activated flash sector protection for our CFI NOR flash chip
> > > (CONFIG_SYS_FLASH_PROTECTION). Protection bits for U-Boot code
> > sectors
> > > and environment sector are set by default during U-Boot startup.
> > > Thus from Linux, I cannot write to this sectors, which is generally
> > > a correct behavior. BUT, with this, changing the environment from
> > > user space with the fw_setenv tool does not work anymore.
> > > And here's the question: How to deal with this issue correctly? Two
> > > approaches came in my mind:
> > > a)
> > > Unprotecting the flash sectors by the Linux kernel (in CFI driver).
> > > This seems not to be implemented yet, at least for AMD CFI command
> > > set (0x0002).
> > > However, when unprotecting flash sectors from Linux side, does a
> > > sector protection actually makes sense?
> > > b)
> > > Introduce a config option in U-Boot which allows to leave the
> > > environment unprotected. See patch below. I called this option
> > > CONFIG_ENV_UNPROTECTED When defined, U-Boot does not protect
> the
> > > environment. For compatibility with boards in which the environment
> > > sector is already protected, I left the unprotect part in the
> > > saveenv function untouched.
> > >
> > > Any comments about this issue?
> > > Thanks and regards
> > > Georg
> > >
> > > Index: common/env_flash.c
> > >
> >
> ==========================================================
> > =========
> > > --- common/env_flash.c	(revision 3966)
> > > +++ common/env_flash.c	(working copy)
> > > @@ -218,9 +218,12 @@
> > >  done:
> > >  	if (saved_data)
> > >  		free(saved_data);
> > > +
> > > +#ifndef CONFIG_ENV_UNPROTECTED
> > >  	/* try to re-protect */
> > >  	flash_sect_protect(1, (ulong)flash_addr, end_addr);
> > >  	flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
> > > +#endif
> > >
> > >  	return rc;
> > >  }
> > > Index: drivers/mtd/cfi_flash.c
> > >
> >
> ==========================================================
> > =========
> > > --- drivers/mtd/cfi_flash.c	(revision 3966)
> > > +++ drivers/mtd/cfi_flash.c	(working copy)
> > > @@ -2294,7 +2294,7 @@
> > >  #endif
> > >
> > >  	/* Environment protection ON by default */ -#ifdef
> > > CONFIG_ENV_IS_IN_FLASH
> > > +#if defined(CONFIG_ENV_IS_IN_FLASH) &&
> > > !defined(CONFIG_ENV_UNPROTECTED)
> > >  	flash_protect(FLAG_PROTECT_SET,
> > >  		       CONFIG_ENV_ADDR,
> > >  		       CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1,
> > >
> > >
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot@lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot@lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot
> > >
> > >
> > >
> >
> >
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

Index: common/env_flash.c
===================================================================
--- common/env_flash.c	(revision 4065)
+++ common/env_flash.c	(working copy)
@@ -218,9 +218,12 @@ 
 done:
 	if (saved_data)
 		free(saved_data);
+
+#ifndef CONFIG_ENV_UNPROTECTED
 	/* try to re-protect */
 	flash_sect_protect(1, (ulong)flash_addr, end_addr);
 	flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
+#endif
 
 	return rc;
 }
@@ -310,7 +313,9 @@ 
 	if (saved_data)
 		free(saved_data);
 	/* try to re-protect */
+#ifndef CONFIG_ENV_UNPROTECTED
 	flash_sect_protect(1, (long)flash_addr, end_addr);
+#endif
 	return rc;
 }
 #endif /* CMD_SAVEENV */
@@ -340,7 +345,9 @@ 
 		flash_write(&flag,
 			    (ulong)&(flash_addr_new->flags),
 			    sizeof(flash_addr_new->flags));
+#ifndef CONFIG_ENV_UNPROTECTED
 		flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
+#endif
 	}
 
 	if (flash_addr->flags != ACTIVE_FLAG &&
@@ -352,7 +359,9 @@ 
 		flash_write(&flag,
 			    (ulong)&(flash_addr->flags),
 			    sizeof(flash_addr->flags));
+#ifndef CONFIG_ENV_UNPROTECTED
 		flash_sect_protect(1, (ulong)flash_addr, end_addr);
+#endif
 	}
 
 	if (gd->env_valid == 2)
Index: drivers/mtd/cfi_flash.c
===================================================================
--- drivers/mtd/cfi_flash.c	(revision 4065)
+++ drivers/mtd/cfi_flash.c	(working copy)
@@ -2294,7 +2294,7 @@ 
 #endif
 
 	/* Environment protection ON by default */
-#ifdef CONFIG_ENV_IS_IN_FLASH
+#if defined(CONFIG_ENV_IS_IN_FLASH) && !defined(CONFIG_ENV_UNPROTECTED)
 	flash_protect(FLAG_PROTECT_SET,
 		       CONFIG_ENV_ADDR,
 		       CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1,
@@ -2302,7 +2302,7 @@ 
 #endif
 
 	/* Redundant environment protection ON by default */
-#ifdef CONFIG_ENV_ADDR_REDUND
+#if defined(CONFIG_ENV_ADDR_REDUND) && !defined(CONFIG_ENV_UNPROTECTED)
 	flash_protect(FLAG_PROTECT_SET,
 		       CONFIG_ENV_ADDR_REDUND,
 		       CONFIG_ENV_ADDR_REDUND + CONFIG_ENV_SECT_SIZE - 1,

Regards
Georg



-----Ursprüngliche Nachricht-----
Von: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] Im Auftrag von Waibel Georg
Gesendet: Dienstag, 26. Februar 2013 12:05
An: 'u-boot@lists.denx.de'
Betreff: [U-Boot] Flash protection and fw_setenv tool

Hi,

I activated flash sector protection for our CFI NOR flash chip (CONFIG_SYS_FLASH_PROTECTION). Protection bits for U-Boot code sectors and environment sector are set by default during U-Boot startup. Thus from Linux, I cannot write to this sectors, which is generally a correct behavior. BUT, with this, changing the environment from user space with the fw_setenv tool does not work anymore. 
And here's the question: How to deal with this issue correctly? Two approaches came in my mind:
a)
Unprotecting the flash sectors by the Linux kernel (in CFI driver). This seems not to be implemented yet, at least for AMD CFI command set (0x0002). 
However, when unprotecting flash sectors from Linux side, does a sector protection actually makes sense?
b)
Introduce a config option in U-Boot which allows to leave the environment unprotected. See patch below. I called this option CONFIG_ENV_UNPROTECTED When defined, U-Boot does not protect the environment. For compatibility with boards in which the environment sector is already protected, I left the unprotect part in the saveenv function untouched.

Any comments about this issue?
Thanks and regards
Georg

Index: common/env_flash.c
===================================================================
--- common/env_flash.c	(revision 3966)
+++ common/env_flash.c	(working copy)
@@ -218,9 +218,12 @@ 
 done:
 	if (saved_data)
 		free(saved_data);
+
+#ifndef CONFIG_ENV_UNPROTECTED
 	/* try to re-protect */
 	flash_sect_protect(1, (ulong)flash_addr, end_addr);
 	flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
+#endif
 
 	return rc;
 }
Index: drivers/mtd/cfi_flash.c
===================================================================
--- drivers/mtd/cfi_flash.c	(revision 3966)
+++ drivers/mtd/cfi_flash.c	(working copy)
@@ -2294,7 +2294,7 @@ 
 #endif
 
 	/* Environment protection ON by default */ -#ifdef CONFIG_ENV_IS_IN_FLASH
+#if defined(CONFIG_ENV_IS_IN_FLASH) && !defined(CONFIG_ENV_UNPROTECTED)
 	flash_protect(FLAG_PROTECT_SET,
 		       CONFIG_ENV_ADDR,
 		       CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1,