public inbox for ecos-devel@sourceware.org
 help / color / mirror / Atom feed
* AW: contributing a failsafe update meachanism for FIS from within ecos applications
@ 2004-10-20 16:14 Neundorf, Alexander
  2004-10-22 19:14 ` Andrew Lunn
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Neundorf, Alexander @ 2004-10-20 16:14 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-devel

[-- Attachment #1: Type: text/plain, Size: 636 bytes --]

Hi,

so here comes a patch with all these changes, including a cdl-entry.
In fis_init() I didn't find the correct place where to erase the additional flash block, there I'd need help.

Main change:

#define EFIS_MAGIC_LENGTH 10
#define EFIS_MAGIC ".FisValid"  //exactly 10 bytes

#define EFIS_VALID       (0xa5a5)
#define EFIS_IN_PROGRESS (0xfdfd)
#define EFIS_EMPTY       (0xffff)

struct fis_valid_info
{
   unsigned char magic_name[EFIS_MAGIC_LENGTH];
   unsigned short valid_flag;
   unsigned long version_count;
};

So it fits again completely in the 16 bytes for the name.

Please comment :-)

Bye
Alex

[-- Attachment #2: fis.fail_save-3.patch --]
[-- Type: application/octet-stream, Size: 8961 bytes --]

diff -rbup current.orig/cdl/redboot.cdl current/cdl/redboot.cdl
--- current.orig/cdl/redboot.cdl	Sun Sep 19 19:49:59 2004
+++ current/cdl/redboot.cdl	Wed Oct 20 18:06:18 2004
@@ -623,6 +623,17 @@ cdl_package CYGPKG_REDBOOT {
                   determine what's free and what's not."
             }
     
+            cdl_option CYGOPT_REDBOOT_ENHANCED_FIS {
+                display         "RedBoot Enhanced Flash Image System support"
+                default_value   0
+                doc             ref/flash-image-system.html
+                description "
+                    This option enables the Enhanced Flash Image System commands
+                    and support within RedBoot.  If enabled a flash block will be
+                  reserved for a second copy of the fis table.
+                  "
+            }
+    
             cdl_component CYGPKG_REDBOOT_FIS_CONTENTS {
                 display       "Flash Image System default directory contents"
                 active_if     CYGOPT_REDBOOT_FIS
@@ -632,6 +643,18 @@ cdl_package CYGPKG_REDBOOT {
                     display         "Flash block containing the Directory"
                     flavor          data
                     default_value   (-1)
+                    description "
+                      Which block of flash should hold the directory 
+                      information. Positive numbers are absolute block numbers. 
+                      Negative block numbers count backwards from the last block.
+                      eg 2 means block 2, -2 means the last but one block."
+                }
+
+                cdl_option CYGNUM_REDBOOT_FIS_ADDITIONAL_DIRECTORY_BLOCK {
+                    display         "Flash block containing the backup Directory"
+                    active_if       CYGOPT_REDBOOT_ENHANCED_FIS
+                    flavor          data
+                    default_value   (-3)
                     description "
                       Which block of flash should hold the directory 
                       information. Positive numbers are absolute block numbers. 
Only in current/cdl: redboot.cdl~
diff -rbup current.orig/include/fis.h current/include/fis.h
--- current.orig/include/fis.h	Sat Aug 24 13:20:55 2002
+++ current/include/fis.h	Wed Oct 20 17:56:25 2004
@@ -73,7 +73,22 @@ struct fis_image_desc {
     unsigned long file_cksum;    // Checksum over image data
 };
 
+#define EFIS_MAGIC_LENGTH 10
+#define EFIS_MAGIC ".FisValid"  //exactly 10 bytes
+
+#define EFIS_VALID       (0xa5a5)
+#define EFIS_IN_PROGRESS (0xfdfd)
+#define EFIS_EMPTY       (0xffff)
+
+struct fis_valid_info
+{
+   unsigned char magic_name[EFIS_MAGIC_LENGTH];
+   unsigned short valid_flag;
+   unsigned long version_count;
+};
+
 struct fis_image_desc *fis_lookup(char *name, int *num);
 
 #endif // CYGOPT_REDBOOT_FIS
 #endif // _FIS_H_
+
diff -rbup current.orig/src/flash.c current/src/flash.c
--- current.orig/src/flash.c	Wed Sep  1 23:21:30 2004
+++ current/src/flash.c	Wed Oct 20 18:03:45 2004
@@ -169,6 +169,8 @@ int flash_block_size, flash_num_blocks;
 #ifdef CYGOPT_REDBOOT_FIS
 void *fis_work_block;
 void *fis_addr;
+void *fis_addr0;
+void *fis_addr1;
 int fisdir_size;  // Size of FIS directory.
 #endif
 #ifdef CYGSEM_REDBOOT_FLASH_CONFIG
@@ -278,6 +280,7 @@ fis_init(int argc, char *argv[])
 {
     int stat;
     struct fis_image_desc *img;
+    struct fis_valid_info* fvi=NULL;
     void *err_addr;
     bool full_init = false;
     struct option_info opts[1];
@@ -301,9 +304,21 @@ fis_init(int argc, char *argv[])
     redboot_image_size = flash_block_size > MIN_REDBOOT_IMAGE_SIZE ? 
                          flash_block_size : MIN_REDBOOT_IMAGE_SIZE;
 
-    // Create a pseudo image for RedBoot
     img = (struct fis_image_desc *)fis_work_block;
     memset(img, 0xFF, fisdir_size);  // Start with erased data
+
+#ifdef CYGOPT_REDBOOT_ENHANCED_FIS
+    //create the valid flag entry
+    fvi=(struct fis_valid_info*)img;
+    memset(img, 0, sizeof(struct fis_image_desc));
+    strcpy(fvi->magic_name, EFIS_MAGIC);
+
+    fvi->valid_flag=EFIS_VALID;
+    fvi->version_count=0;
+    img++;
+#endif
+
+    // Create a pseudo image for RedBoot
 #ifdef CYGOPT_REDBOOT_FIS_RESERVED_BASE
     memset(img, 0, sizeof(*img));
     strcpy(img->name, "(reserved)");
@@ -357,10 +372,18 @@ fis_init(int argc, char *argv[])
     // And a descriptor for the descriptor table itself
     memset(img, 0, sizeof(*img));
     strcpy(img->name, "FIS directory");
-    img->flash_base = (CYG_ADDRESS)fis_addr;
-    img->mem_base = (CYG_ADDRESS)fis_addr;
+    img->flash_base = (CYG_ADDRESS)fis_addr0;
+    img->mem_base = (CYG_ADDRESS)fis_addr0;
+    img->size = fisdir_size;
+    img++;
+#ifdef CYGOPT_REDBOOT_ENHANCED_FIS
+    memset(img, 0, sizeof(*img));
+    strcpy(img->name, "FIS directory~");
+    img->flash_base = (CYG_ADDRESS)fis_addr1;
+    img->mem_base = (CYG_ADDRESS)fis_addr1;
     img->size = fisdir_size;
     img++;
+#endif
 
 #ifdef CYGOPT_REDBOOT_FIS_DIRECTORY_ARM_SIB_ID
     // FIS gets the size of a full block - note, this should be changed
@@ -1351,10 +1374,36 @@ _flash_info(void)
            flash_start, (CYG_ADDRWORD)flash_end + 1, flash_num_blocks, (void *)flash_block_size);
 }
 
+#ifdef CYGOPT_REDBOOT_ENHANCED_FIS
+static int
+get_valid_buf(struct fis_valid_info* fvi0, struct fis_valid_info* fvi1)
+{
+   if (strncmp(fvi0->magic_name, EFIS_MAGIC, EFIS_MAGIC_LENGTH)!=0)  //buf1 must be valid
+      return 1;
+   else if (strncmp(fvi1->magic_name, EFIS_MAGIC, EFIS_MAGIC_LENGTH)!=0) //buf0 must be valid
+      return 0;
+
+   //magic is ok for both, now check the valid flag
+   if (fvi0->valid_flag!=EFIS_VALID) //buf1 must be valid
+      return 1;
+   else if (fvi1->valid_flag!=EFIS_VALID) //buf0 must be valid
+      return 0;
+
+   //now check the version
+   if (fvi1->version_count == (fvi0->version_count+1)) //buf1 must be valid
+      return 1;
+
+   return 0;
+}
+#endif
+
 bool
 do_flash_init(void)
 {
     int stat;
+    void *err_addr=0;
+    struct fis_valid_info fvi0;
+    struct fis_valid_info fvi1;
 
     if (!__flash_init) {
         __flash_init = 1;
@@ -1379,17 +1428,64 @@ do_flash_init(void)
         workspace_end = (unsigned char *)(workspace_end-fisdir_size);
         fis_work_block = workspace_end;
 # endif
+
+        fis_addr0 = NULL;
+        fis_addr1 = NULL;
+
         if (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK < 0) {
-            fis_addr = (void *)((CYG_ADDRESS)flash_end + 1 +
+            fis_addr0 = (void *)((CYG_ADDRESS)flash_end + 1 +
                                 (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK*flash_block_size));
         } else {
-            fis_addr = (void *)((CYG_ADDRESS)flash_start + 
+            fis_addr0 = (void *)((CYG_ADDRESS)flash_start +
                                 (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK*flash_block_size));
         }
-        if (((CYG_ADDRESS)fis_addr + fisdir_size - 1) > (CYG_ADDRESS)flash_end) {
+
+#ifdef CYGOPT_REDBOOT_ENHANCED_FIS
+        if (CYGNUM_REDBOOT_FIS_ADDITIONAL_DIRECTORY_BLOCK < 0) {
+           fis_addr1 = (void *)((CYG_ADDRESS)flash_end + 1 +
+                                (CYGNUM_REDBOOT_FIS_ADDITIONAL_DIRECTORY_BLOCK*flash_block_size));
+        } else {
+           fis_addr1 = (void *)((CYG_ADDRESS)flash_start +
+                                (CYGNUM_REDBOOT_FIS_ADDITIONAL_DIRECTORY_BLOCK*flash_block_size));
+        }
+
+        if ((((CYG_ADDRESS)fis_addr0 + fisdir_size - 1) > (CYG_ADDRESS)flash_end)
+            || (((CYG_ADDRESS)fis_addr1 + fisdir_size - 1) > (CYG_ADDRESS)flash_end)
+            || (fis_addr0==fis_addr1)) {
             diag_printf("FIS directory doesn't fit\n");
             return false;
         }
+
+        FLASH_READ(fis_addr0, &fvi0, sizeof(struct fis_valid_info), (void **)&err_addr);
+        FLASH_READ(fis_addr1, &fvi1, sizeof(struct fis_valid_info), (void **)&err_addr);
+
+#ifdef REDBOOT_FLASH_REVERSE_BYTEORDER
+        fvi0.magic=CYG_SWAP32(fvi0.magic);
+        fvi0.valid_flag=CYG_SWAP32(fvi0.valid_flag);
+        fvi0.version_count=CYG_SWAP32(fvi0.version_count);
+        fvi1.magic=CYG_SWAP32(fvi1.magic);
+        fvi1.valid_flag=CYG_SWAP32(fvi1.valid_flag);
+        fvi1.version_count=CYG_SWAP32(fvi1.version_count);
+#endif
+
+        if ((strncmp(fvi0.magic_name, EFIS_MAGIC, EFIS_MAGIC_LENGTH)==0) || (memcmp(fvi1.magic_name, EFIS_MAGIC, EFIS_MAGIC_LENGTH)==0)) {
+           if (get_valid_buf(&fvi0, &fvi1)==0)
+              fis_addr=fis_addr0;
+           else
+              fis_addr=fis_addr1;
+        } else {
+           fis_addr1=NULL;
+           fis_addr=fis_addr0;
+        }
+#else  //no enhanced fis
+        if (((CYG_ADDRESS)fis_addr0 + fisdir_size - 1) > (CYG_ADDRESS)flash_end) {
+            diag_printf("FIS directory doesn't fit\n");
+            return false;
+        }
+        fis_addr1=NULL;
+        fis_addr=fis_addr0;
+#endif
+
         fis_read_directory();
 #endif
     }
Only in current/src: flash.c~

[-- Attachment #3: fisfs-3.tar.gz --]
[-- Type: application/x-gzip, Size: 8722 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread
* AW: contributing a failsafe update meachanism for FIS from within ecos applications
@ 2004-10-19 15:27 Neundorf, Alexander
  2004-10-19 16:13 ` Andrew Lunn
  0 siblings, 1 reply; 13+ messages in thread
From: Neundorf, Alexander @ 2004-10-19 15:27 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-devel

[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]

Hi all,

> Von: Andrew Lunn [mailto:andrew@lunn.ch]
> Betreff: Re: contributing a failsafe update meachanism for FIS from
> within ecos applications
> 
> > Ok, I'll post a modified patch which uses "$Magic[01]" as "magic
> > number" in the next days, with "$Magic0" for the fis table 0 and
> > "$Magic1" for fis table 1. This leaves 8 bytes room for the
> > valid_flag and the version_count.
> 
> The problem with "Magic" is that it does not indicate what its for. If
> you don't know what its for, somebody will delete it. "fis valid" is
> less likely to be deleted since it sounds more important. I also don't
> see why you need Magic0 and Magic1. 
> 
> You don't need 8 bytes. All you really need is 4 bits. 2 bits for
> valid, in progress and empty, plus 2 bits for the version. 


No two bits for "valid or in_progress or empty" is not enough. In the case that power is lost while writing exactly these two bits the state of these two bits is undefined. So the probability that they end up as valid when I actually wanted to write in_progress is 25%, at least >>0. If the valid_flag is 32 bits and only one combination is considered valid, then the probability is 1/(2**32).
Two bits for the version are with my proposed scheme also not enough. The old table will never be touched, the new one will increase the version of the old table by one. So no wrap-around may happen. This won't happen with 32 bits.

So, here comes a modified patch. 

With the basic changes being:

#define EFIS_VALID "$_FisValid_"

struct fis_valid_info
{
   char magic_name[12];
   unsigned long valid_flag;
   CYG_ADDRESS unused_flash_base;
   unsigned long version_count;
};

This will ensure that also older redboots will recognize this entry in the table as being used but consuming no space on the flash (length and flash_base ==0).
$_FisValid_ isn't listed by "fis list" since its address isn't found (see the loop in fis list).
The flash block of the second fis table is now defined with a cdl-option, I didn't get around yet to define an option to switch the second table on and off.
The fisfs-2.tar.gz contains the changes required in the application code. Still no ecos fs (but will come).

Bye
Alex


[-- Attachment #2: fisfs-2.tar.gz --]
[-- Type: application/x-gzip, Size: 8791 bytes --]

[-- Attachment #3: fis.fail_save.patch --]
[-- Type: application/octet-stream, Size: 7485 bytes --]

diff -rbup current.orig/cdl/redboot.cdl current/cdl/redboot.cdl
--- current.orig/cdl/redboot.cdl	Sun Sep 19 19:49:59 2004
+++ current/cdl/redboot.cdl	Tue Oct 19 17:16:18 2004
@@ -638,6 +638,19 @@ cdl_package CYGPKG_REDBOOT {
                       Negative block numbers count backwards from the last block.
                       eg 2 means block 2, -2 means the last but one block."
                 }
+
+					  cdl_option CYGNUM_REDBOOT_FIS_ADDITIONAL_DIRECTORY_BLOCK {
+                    display         "Flash block containing the backup Directory"
+                    flavor          data
+                    default_value   (-1)
+                    description "
+                      Which block of flash should hold the directory
+                      information. Positive numbers are absolute block numbers.
+                      Negative block numbers count backwards from the last block.
+                      eg 2 means block 2, -2 means the last but one block."
+                  }
+
+
     
                 cdl_option CYGOPT_REDBOOT_FIS_RESERVED_BASE {
                     display         "Pseudo-file to describe reserved area"
Only in current/cdl: redboot.cdl~
diff -rbup current.orig/include/fis.h current/include/fis.h
--- current.orig/include/fis.h	Sat Aug 24 13:20:55 2002
+++ current/include/fis.h	Tue Oct 19 17:17:02 2004
@@ -73,7 +73,22 @@ struct fis_image_desc {
     unsigned long file_cksum;    // Checksum over image data
 };
 
+#define EFIS_MAGIC "$_FisValid_"  //exactly 12 bytes
+
+#define EFIS_VALID       (0xa5a5a5a5)
+#define EFIS_IN_PROGRESS (0xfdfdfdfd)
+#define EFIS_EMPTY       (0xffffffff)
+
+struct fis_valid_info
+{
+   unsigned char magic_name[12];
+   unsigned long valid_flag;
+   CYG_ADDRESS unused_flash_base;
+   unsigned long version_count;
+};
+
 struct fis_image_desc *fis_lookup(char *name, int *num);
 
 #endif // CYGOPT_REDBOOT_FIS
 #endif // _FIS_H_
+
Only in current/include: fis.h~
diff -rbup current.orig/src/flash.c current/src/flash.c
--- current.orig/src/flash.c	Wed Sep  1 23:21:30 2004
+++ current/src/flash.c	Tue Oct 19 17:15:26 2004
@@ -169,6 +169,8 @@ int flash_block_size, flash_num_blocks;
 #ifdef CYGOPT_REDBOOT_FIS
 void *fis_work_block;
 void *fis_addr;
+void *fis_addr0;
+void *fis_addr1;
 int fisdir_size;  // Size of FIS directory.
 #endif
 #ifdef CYGSEM_REDBOOT_FLASH_CONFIG
@@ -278,6 +280,7 @@ fis_init(int argc, char *argv[])
 {
     int stat;
     struct fis_image_desc *img;
+    struct fis_valid_info* fvi=NULL;
     void *err_addr;
     bool full_init = false;
     struct option_info opts[1];
@@ -301,9 +304,19 @@ fis_init(int argc, char *argv[])
     redboot_image_size = flash_block_size > MIN_REDBOOT_IMAGE_SIZE ? 
                          flash_block_size : MIN_REDBOOT_IMAGE_SIZE;
 
-    // Create a pseudo image for RedBoot
     img = (struct fis_image_desc *)fis_work_block;
     memset(img, 0xFF, fisdir_size);  // Start with erased data
+
+    //create the valid flag entry
+    fvi=(struct fis_valid_info*)img;
+    memset(img, 0, sizeof(struct fis_image_desc));
+    memcpy(fvi->magic_name, EFIS_MAGIC, 12);
+
+    fvi->valid_flag=EFIS_VALID;
+    fvi->version_count=0;
+    img++;
+
+    // Create a pseudo image for RedBoot
 #ifdef CYGOPT_REDBOOT_FIS_RESERVED_BASE
     memset(img, 0, sizeof(*img));
     strcpy(img->name, "(reserved)");
@@ -357,8 +370,14 @@ fis_init(int argc, char *argv[])
     // And a descriptor for the descriptor table itself
     memset(img, 0, sizeof(*img));
     strcpy(img->name, "FIS directory");
-    img->flash_base = (CYG_ADDRESS)fis_addr;
-    img->mem_base = (CYG_ADDRESS)fis_addr;
+    img->flash_base = (CYG_ADDRESS)fis_addr0;
+    img->mem_base = (CYG_ADDRESS)fis_addr0;
+    img->size = fisdir_size;
+    img++;
+    memset(img, 0, sizeof(*img));
+    strcpy(img->name, "FIS directory~");
+    img->flash_base = (CYG_ADDRESS)fis_addr1;
+    img->mem_base = (CYG_ADDRESS)fis_addr1;
     img->size = fisdir_size;
     img++;
 
@@ -1351,10 +1370,35 @@ _flash_info(void)
            flash_start, (CYG_ADDRWORD)flash_end + 1, flash_num_blocks, (void *)flash_block_size);
 }
 
+static int
+get_valid_buf(struct fis_valid_info* fvi0, struct fis_valid_info* fvi1)
+{
+   if (memcmp(fvi0->magic_name, EFIS_MAGIC, 12)!=0)  //buf1 must be valid
+      return 1;
+   else if (memcmp(fvi1->magic_name, EFIS_MAGIC, 12)!=0) //buf0 must be valid
+      return 0;
+
+   //magic is ok for both, now check the valid flag
+   if (fvi0->valid_flag!=EFIS_VALID) //buf1 must be valid
+      return 1;
+   else if (fvi1->valid_flag!=EFIS_VALID) //buf0 must be valid
+      return 0;
+
+   //now check the version
+   if ((fvi1->version_count > fvi0->version_count) //buf1 must be valid
+       && (fvi1->version_count != 0xffffffff))
+      return 1;
+
+   return 0;
+}
+
 bool
 do_flash_init(void)
 {
     int stat;
+    void *err_addr=0;
+    struct fis_valid_info fvi0;
+    struct fis_valid_info fvi1;
 
     if (!__flash_init) {
         __flash_init = 1;
@@ -1379,17 +1423,55 @@ do_flash_init(void)
         workspace_end = (unsigned char *)(workspace_end-fisdir_size);
         fis_work_block = workspace_end;
 # endif
+
+        fis_addr0 = NULL;
+        fis_addr1 = NULL;
+
         if (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK < 0) {
-            fis_addr = (void *)((CYG_ADDRESS)flash_end + 1 +
+            fis_addr0 = (void *)((CYG_ADDRESS)flash_end + 1 +
                                 (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK*flash_block_size));
         } else {
-            fis_addr = (void *)((CYG_ADDRESS)flash_start + 
+            fis_addr0 = (void *)((CYG_ADDRESS)flash_start +
                                 (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK*flash_block_size));
         }
-        if (((CYG_ADDRESS)fis_addr + fisdir_size - 1) > (CYG_ADDRESS)flash_end) {
+
+        if (CYGNUM_REDBOOT_FIS_ADDITIONAL_DIRECTORY_BLOCK < 0) {
+           fis_addr1 = (void *)((CYG_ADDRESS)flash_end + 1 +
+                                (CYGNUM_REDBOOT_FIS_ADDITIONAL_DIRECTORY_BLOCK*flash_block_size));
+        } else {
+           fis_addr1 = (void *)((CYG_ADDRESS)flash_start +
+                                (CYGNUM_REDBOOT_FIS_ADDITIONAL_DIRECTORY_BLOCK*flash_block_size));
+        }
+
+        if ((((CYG_ADDRESS)fis_addr0 + fisdir_size - 1) > (CYG_ADDRESS)flash_end)
+            || (((CYG_ADDRESS)fis_addr1 + fisdir_size - 1) > (CYG_ADDRESS)flash_end)
+            || (fis_addr0==fis_addr1)) {
             diag_printf("FIS directory doesn't fit\n");
             return false;
         }
+
+        FLASH_READ(fis_addr0, &fvi0, sizeof(struct fis_valid_info), (void **)&err_addr);
+        FLASH_READ(fis_addr1, &fvi1, sizeof(struct fis_valid_info), (void **)&err_addr);
+
+#ifdef REDBOOT_FLASH_REVERSE_BYTEORDER
+        fvi0.magic=CYG_SWAP32(fvi0.magic);
+        fvi0.valid_flag=CYG_SWAP32(fvi0.valid_flag);
+        fvi0.version_count=CYG_SWAP32(fvi0.version_count);
+        fvi1.magic=CYG_SWAP32(fvi1.magic);
+        fvi1.valid_flag=CYG_SWAP32(fvi1.valid_flag);
+        fvi1.version_count=CYG_SWAP32(fvi1.version_count);
+#endif
+
+        if ((memcmp(fvi0.magic_name, EFIS_MAGIC, 12)==0) || (memcmp(fvi1.magic_name, EFIS_MAGIC, 12)==0)) {
+           if (get_valid_buf(&fvi0, &fvi1)==0)
+              fis_addr=fis_addr0;
+           else
+              fis_addr=fis_addr1;
+        } else {
+           fis_addr1=NULL;
+           fis_addr=fis_addr0;
+        }
+
         fis_read_directory();
 #endif
     }
Only in current/src: flash.c~

^ permalink raw reply	[flat|nested] 13+ messages in thread
* AW: contributing a failsafe update meachanism for FIS from within ecos applications
@ 2004-10-18 14:04 Neundorf, Alexander
  2004-10-18 14:49 ` Andrew Lunn
  0 siblings, 1 reply; 13+ messages in thread
From: Neundorf, Alexander @ 2004-10-18 14:04 UTC (permalink / raw)
  To: Andrew Lunn, ecos-devel


> -----Ursprüngliche Nachricht-----
> Von: Andrew Lunn [mailto:andrew@lunn.ch]
> Gesendet: Montag, 18. Oktober 2004 14:53
> An: Neundorf, Alexander
> Cc: ecos-devel@sources.redhat.com
> Betreff: Re: contributing a failsafe update meachanism for FIS from
> within ecos applications
> 
> 
> On Mon, Oct 18, 2004 at 02:25:53PM +0200, Neundorf, Alexander wrote:
> > Any technical comments ?
> 
> You need to make some of the code conditionally compiled on CDL. ie by
> default there is no valid marker, only one flash block is used etc. 

Yes.
 
> You should have CDL to control where the second flash block is. Its
> bad to assume its the next block. 

Really ?
IMO it would be ok if the blocks are directly one after the other. One could think of it as a fis dir twice as big as before containing two copies of this fis dir.
 
> What happens when an old redboot reads the fis directory of a new
> redboot with these valid/invalid entry? 

If a new redboot reads an old fis dir everything should be ok. If not, then I've overlooked something.

If an old redboot reads a new fis dir there will be some problems.
The "valid marker" entry will be considered empty. 
So it won't show up on "fis list". Since it doesn't allocate space in the flash, the space allocation algorithm will work correctly. There will be a problem with "fis create" when an empty entry in the fis table is searched, the "valid marker" entry might be overwritten.
But how can this happen ? A new fis dir can only be created using a new redboot. Why should after a new fis dir has been created using a new redboot an old redboot run on this flash ?

> Does the entry show up on the fis list? 

No, it's hidden. Should it be visible ?

> Does it have a sensible name?

Since it's hidden it currently doesn't have a name. With my current approach a name is slightly complicated, since I use the first bytes of the name for the valid marker.
Maybe I could change the "magic number" from "0xff1234ff" to "$VALID0\0" and "$VALID1\0".
This would still leave room for 2 integers (valid_flag and version_count) and be even more compatible with the current redboot. It would be listed by fis list, not overwritten by an old redboot, and via the last digit you could even recognize which one is currently active. OTOH this would make it impossible to use "$VALID[01]" for "normal" fis images (which might exist right now, at least theoretically).

> Is there any way to see if its valid/invalid etc.

What do you suggest ?
This mean adding commands to fis. Until now I wanted to avoid this and stay completely compatible on the outside. 
 
> At the moment you seemed to of focused on the mechanics of how you
> boot/update reliably. Thats the easy part. 
> You now need to see how it fits into the rest of redboot and eCos. 
> Hopefully some of the
> questions i asked above with get you started in that direction.

Yes, that's why I asked. What I need is that we reach a consensus on the layout of the data on the flash.

Bye
Alex

^ permalink raw reply	[flat|nested] 13+ messages in thread
* AW: contributing a failsafe update meachanism for FIS from within ecos applications
@ 2004-10-18 12:26 Neundorf, Alexander
  2004-10-18 12:52 ` Andrew Lunn
  0 siblings, 1 reply; 13+ messages in thread
From: Neundorf, Alexander @ 2004-10-18 12:26 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-devel

> Von: Andrew Lunn [mailto:andrew@lunn.ch]
> Gesendet: Montag, 18. Oktober 2004 14:05
> An: Neundorf, Alexander
> Cc: ecos-devel@sources.redhat.com
> Betreff: Re: contributing a failsafe update meachanism for FIS from
> within ecos applications
> 
> 
> > Ok, I already had a quick look at the ecos and ecoscentrics web
> > pages but didn't find information about the copyright
> > assignement. Can you give me a link on the procedure ?
> 
> http://ecos.sourceware.org/assign.html
> 
>         Andrew

Ok.

Any technical comments ?

Bye
Alex

^ permalink raw reply	[flat|nested] 13+ messages in thread
* AW: contributing a failsafe update meachanism for FIS from within ecos applications
@ 2004-10-18 12:02 Neundorf, Alexander
  2004-10-18 12:05 ` Andrew Lunn
  0 siblings, 1 reply; 13+ messages in thread
From: Neundorf, Alexander @ 2004-10-18 12:02 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-devel



> -----Ursprüngliche Nachricht-----
> Von: Andrew Lunn [mailto:andrew@lunn.ch]
> Gesendet: Montag, 18. Oktober 2004 13:16
> An: Neundorf, Alexander
> Cc: ecos-devel@sources.redhat.com
> Betreff: Re: contributing a failsafe update meachanism for FIS from
> within ecos applications
> 
> 
> On Mon, Oct 18, 2004 at 01:05:06PM +0200, Neundorf, Alexander wrote:
> > Hi,
> > 
> > I already posted to ecos-discuss, but without much response.
> 
> Actually, Gary is waiting for a copyright assignment from you, plus he
> has just moved house so i guess there is some degree of chaos
> involved.
> 
> Once the copyright assignment is sorted out we can take this further.
> Until then i doubt anything will make it into anoncvs.

Ok, I already had a quick look at the ecos and ecoscentrics web pages but didn't find information about the copyright assignement. Can you give me a link on the procedure ?

Bye
Alex

^ permalink raw reply	[flat|nested] 13+ messages in thread
* contributing a failsafe update meachanism for FIS from within ecos applications
@ 2004-10-18 11:05 Neundorf, Alexander
  2004-10-18 11:16 ` Andrew Lunn
  2004-10-18 20:25 ` Andrew Lunn
  0 siblings, 2 replies; 13+ messages in thread
From: Neundorf, Alexander @ 2004-10-18 11:05 UTC (permalink / raw)
  To: ecos-devel

[-- Attachment #1: Type: text/plain, Size: 5676 bytes --]

Hi,

I already posted to ecos-discuss, but without much response.

We need to be able to perform safe updates of the firmware, safe regarding power loss at any point in time. Since redboot comes with FIS, we'd like to use fis.
In order to update the firmware a new firmware image has to be placed on the flash and the fis directory has to be updated. When updating the fis directory, the directory is erased and afterwards written with the new contents.
Now if the power goes down directly after erasing the directory redboot can't start the firmware image anymore since it can't read the directory.

In order to enable failsafe operation of redboot and fis under such circumstances, a backup of the fis directory has to be kept until the new directory has been written successfully.
Here comes my proposed strategy:
Currently the fis directory occupies one block of the flash. For safe operation it needs a second block. Both blocks contain the fis directory, but only one is valid (and current).
In the attached patch these blocks are called block 0 (this one existed until now) and block 1 (this one is the new additional one).
Redboot needs a way to determine which block contains the valid information.
For this and to stay compatible with existing flash, I suggest to use the first entry of the fis directory table as a valid marker, which can be used to decide which of the two blocks is valid.
It looks like this:

//1st and 4th byte should be 0xff the two middle bytes != 0xff (endianess)
#define EFIS_MAGIC (0xff1234ff)

#define EFIS_VALID       (0xa5a5a5a5)
#define EFIS_IN_PROGRESS (0xfdfdfdfd)
#define EFIS_EMPTY       (0xffffffff)

struct fis_valid_info
{
   unsigned int magic;          
   unsigned int valid_flag;
   unsigned int version_count;
};

The "magic" is used to check if this fis directory contains a fis_valid_info. 0xff1234ff is constructed this way in order to be compatible with the rest of the algorithms in redboot, which use the first (two) byte(s) of the name to check if the entry is empty. So this entry can't be mistaken as an entry describing an image on the flash.
If the magic matches, the valid_flag is evaluated.
If it is equal to EFIS_VALID then this directory is valid. If both fis directories (from both blocks) have the correct magic and are valid, the version_count comes into play.
The fis directory with the higher version_count will be considered as the most recent valid fis directory and thus be used.

When performing a safe update, the algorithm must do the following:
(after the * followes what happens when the power goes down at this point in time)

1. modify the fis directory (in RAM) so that it reflects the desired changes, set the valid_flag to EFIS_IN_PROGRESS and set version_count=version_count+1;
*nothing has changed yet, so redboot will work as before

2. erase the flash where the currently invalid fis directory is located
*the valid_flag of the fis directory which will become the new valid directory is 0xffffffff, and the valid flag of the currently still active directory is still 0xa5a5a5a5, and the images haven't been touched yet, so still everything ok for redboot

3. write the modified fis directory in this erased flash block.   
*as above, but the valid_flag of the directory which is intended to become valid is now 0xfdfdfdfd. The images still haven't been touched, so everything is ok.

4. modify the flash image (erase, program)
*now the image has been modified. If you erase the only runnable firmware image on the flash you are of course lost, just avoid this. In all other cases, there is still a working fis directory and a working firmware image on the flash. The old current fis directory is still valid, and the currently running firmware image hasn't been touched. By checking the crc's of the images later you can detect which images are broken.

5. after the image is written, set the valid_flag of the fis directory which will become active to 0xa5a5a5a5a5. In order to do this, the flash block doesn't have to be erased, since the transition from 0xfdfdfdfd to 0xa5a5a5a5 only sets some bits to 0. When this is done, the image has been written correctly and the new fis directory has the right magic, the right valid_flag and its version_count is higher than the version_count of the old fis directory.
*if the power goes down while writing the 4 bytes of the valid_flag, either the valid_flag has already reached 0xa5a5a5a5, then everything is ok, if not it will have a valid_flag != 0xa5a5a5a5 and thus not be considered valid.

The attached patch implements support for this strategy in redboot. It basically reads the first entry of both fis blocks, checks them and sets one to be the valid one. The redboot fis commands still behave as always, they don't do the fancy algorithm as described. This is ok since no customer of a device will perform the update directly via fis commands, and if he does, he can simply rebuild the fis with the fis commands.

The other attached file fisfs.tar.gz contains the current working version of the fisfs implementation for ecos applications. I didn't find the time yer to turn this into a real ecos filesystem, but this will happen til the end of this year.
You can have a look at fisfs.cpp, e.g. the function eraseImage() to see an implementation of this strategy.

Since we would like to use this strategy, it would be nice if fis.patch could be applied to ecos cvs, so that we can rely on the binary format on the flash. I guess at least the creation of the fis_valid_info entry in fis_init should be #ifdef'ed with a config switch.

So, what do you think ?

Bye
Alex

[-- Attachment #2: fis.patch --]
[-- Type: application/octet-stream, Size: 6314 bytes --]

diff -rbup current.orig/include/fis.h current/include/fis.h
--- current.orig/include/fis.h	Sat Aug 24 13:20:55 2002
+++ current/include/fis.h	Fri Oct  8 15:23:05 2004
@@ -75,5 +75,19 @@ struct fis_image_desc {
 
 struct fis_image_desc *fis_lookup(char *name, int *num);
 
+//1st and 4th byte should be 0xff the two middle bytes != 0xff (endianess)
+#define EFIS_MAGIC (0xff1234ff)
+
+#define EFIS_VALID       (0xa5a5a5a5)
+#define EFIS_IN_PROGRESS (0xfdfdfdfd)
+#define EFIS_EMPTY       (0xffffffff)
+
+struct fis_valid_info
+{
+   unsigned int magic;
+   unsigned int valid_flag;
+   unsigned int version_count;
+};
+
 #endif // CYGOPT_REDBOOT_FIS
 #endif // _FIS_H_
Only in current/include: fis.h~
diff -rbup current.orig/src/flash.c current/src/flash.c
--- current.orig/src/flash.c	Wed Sep  1 23:21:30 2004
+++ current/src/flash.c	Fri Oct  8 15:39:46 2004
@@ -169,6 +169,8 @@ int flash_block_size, flash_num_blocks;
 #ifdef CYGOPT_REDBOOT_FIS
 void *fis_work_block;
 void *fis_addr;
+void *fis_addr0;
+void *fis_addr1;
 int fisdir_size;  // Size of FIS directory.
 #endif
 #ifdef CYGSEM_REDBOOT_FLASH_CONFIG
@@ -278,6 +280,7 @@ fis_init(int argc, char *argv[])
 {
     int stat;
     struct fis_image_desc *img;
+    struct fis_valid_info* fvi=NULL;
     void *err_addr;
     bool full_init = false;
     struct option_info opts[1];
@@ -301,9 +304,17 @@ fis_init(int argc, char *argv[])
     redboot_image_size = flash_block_size > MIN_REDBOOT_IMAGE_SIZE ? 
                          flash_block_size : MIN_REDBOOT_IMAGE_SIZE;
 
-    // Create a pseudo image for RedBoot
     img = (struct fis_image_desc *)fis_work_block;
     memset(img, 0xFF, fisdir_size);  // Start with erased data
+
+    //create the valid flag entry
+    fvi=(struct fis_valid_info*)img;
+    fvi->magic=EFIS_MAGIC;
+    fvi->valid_flag=EFIS_VALID;
+    fvi->version_count=0;
+    img++;
+
+    // Create a pseudo image for RedBoot
 #ifdef CYGOPT_REDBOOT_FIS_RESERVED_BASE
     memset(img, 0, sizeof(*img));
     strcpy(img->name, "(reserved)");
@@ -357,9 +368,9 @@ fis_init(int argc, char *argv[])
     // And a descriptor for the descriptor table itself
     memset(img, 0, sizeof(*img));
     strcpy(img->name, "FIS directory");
-    img->flash_base = (CYG_ADDRESS)fis_addr;
-    img->mem_base = (CYG_ADDRESS)fis_addr;
-    img->size = fisdir_size;
+    img->flash_base = (CYG_ADDRESS)fis_addr0;
+    img->mem_base = (CYG_ADDRESS)fis_addr0;
+    img->size = fisdir_size*2;
     img++;
 
 #ifdef CYGOPT_REDBOOT_FIS_DIRECTORY_ARM_SIB_ID
@@ -902,7 +913,7 @@ fis_create(int argc, char *argv[])
         // If not image by that name, try and find an empty slot
         img = (struct fis_image_desc *)fis_work_block;
         for (i = 0;  i < fisdir_size/sizeof(*img);  i++, img++) {
-            if (img->name[0] == (unsigned char)0xFF) {
+            if ((img->name[0] == (unsigned char)0xFF) && (img->name[1] == (unsigned char)0xFF)) {
                 break;
             }
         }
@@ -1007,6 +1018,7 @@ fis_delete(int argc, char *argv[])
         diag_printf("Error erasing at %p: %s\n", err_addr, flash_errmsg(stat));
     } else {
         img->name[0] = (unsigned char)0xFF;    
+        img->name[1] = (unsigned char)0xFF;
         fis_update_directory();
     }
 }
@@ -1351,10 +1363,35 @@ _flash_info(void)
            flash_start, (CYG_ADDRWORD)flash_end + 1, flash_num_blocks, (void *)flash_block_size);
 }
 
+static int
+get_valid_buf(struct fis_valid_info* fvi0, struct fis_valid_info* fvi1)
+{
+   if (fvi0->magic!=EFIS_MAGIC)  //buf1 must be valid
+      return 1;
+   else if (fvi1->magic!=EFIS_MAGIC) //buf0 must be valid
+      return 0;
+
+   //magic is ok for both, now check the valid flag
+   if (fvi0->valid_flag!=EFIS_VALID) //buf1 must be valid
+      return 1;
+   else if (fvi1->valid_flag!=EFIS_VALID) //buf0 must be valid
+      return 0;
+
+   //now check the version
+   if ((fvi1->version_count > fvi0->version_count) //buf1 must be valid
+       && (fvi1->version_count != 0xffffffff))
+      return 1;
+
+   return 0;
+}
+
 bool
 do_flash_init(void)
 {
     int stat;
+    void *err_addr=0;
+    struct fis_valid_info fvi0;
+    struct fis_valid_info fvi1;
 
     if (!__flash_init) {
         __flash_init = 1;
@@ -1379,17 +1416,47 @@ do_flash_init(void)
         workspace_end = (unsigned char *)(workspace_end-fisdir_size);
         fis_work_block = workspace_end;
 # endif
+
+        fis_addr0 = NULL;
+        fis_addr1 = NULL;
+
         if (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK < 0) {
-            fis_addr = (void *)((CYG_ADDRESS)flash_end + 1 +
+            fis_addr0 = (void *)((CYG_ADDRESS)flash_end + 1 +
                                 (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK*flash_block_size));
         } else {
-            fis_addr = (void *)((CYG_ADDRESS)flash_start + 
+            fis_addr0 = (void *)((CYG_ADDRESS)flash_start +
                                 (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK*flash_block_size));
         }
+        fis_addr1 = (void *)((CYG_ADDRESS)fis_addr0 + flash_block_size);
+
+
         if (((CYG_ADDRESS)fis_addr + fisdir_size - 1) > (CYG_ADDRESS)flash_end) {
             diag_printf("FIS directory doesn't fit\n");
             return false;
         }
+
+        FLASH_READ(fis_addr0, &fvi0, sizeof(struct fis_valid_info), (void **)&err_addr);
+        FLASH_READ(fis_addr1, &fvi1, sizeof(struct fis_valid_info), (void **)&err_addr);
+
+#ifdef REDBOOT_FLASH_REVERSE_BYTEORDER
+        fvi0.magic=CYG_SWAP32(fvi0.magic);
+        fvi0.valid_flag=CYG_SWAP32(fvi0.valid_flag);
+        fvi0.version_count=CYG_SWAP32(fvi0.version_count);
+        fvi1.magic=CYG_SWAP32(fvi1.magic);
+        fvi1.valid_flag=CYG_SWAP32(fvi1.valid_flag);
+        fvi1.version_count=CYG_SWAP32(fvi1.version_count);
+#endif
+
+        if ((fvi0.magic==EFIS_MAGIC) || (fvi1.magic==EFIS_MAGIC)) {
+           if (get_valid_buf(&fvi0, &fvi1)==0)
+              fis_addr=fis_addr0;
+           else
+              fis_addr=fis_addr1;
+        } else {
+           fis_addr1=NULL;
+           fis_addr=fis_addr0;
+        }
+
         fis_read_directory();
 #endif
     }
Only in current/src: flash.c~

[-- Attachment #3: fisfs.tar.gz --]
[-- Type: application/x-gzip, Size: 8713 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2004-10-25 13:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-10-20 16:14 AW: contributing a failsafe update meachanism for FIS from within ecos applications Neundorf, Alexander
2004-10-22 19:14 ` Andrew Lunn
2004-10-25  7:26   ` Andrew Lunn
2004-10-25  7:32 ` AW: " Slawek
2004-10-25 13:15 ` Redboot lockups Curtis Whitley
2004-10-25 13:26   ` Andrew Lunn
  -- strict thread matches above, loose matches on Subject: below --
2004-10-19 15:27 AW: contributing a failsafe update meachanism for FIS from within ecos applications Neundorf, Alexander
2004-10-19 16:13 ` Andrew Lunn
2004-10-18 14:04 AW: " Neundorf, Alexander
2004-10-18 14:49 ` Andrew Lunn
2004-10-18 12:26 AW: " Neundorf, Alexander
2004-10-18 12:52 ` Andrew Lunn
2004-10-18 12:02 AW: " Neundorf, Alexander
2004-10-18 12:05 ` Andrew Lunn
2004-10-18 11:05 Neundorf, Alexander
2004-10-18 11:16 ` Andrew Lunn
2004-10-18 20:25 ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).