AW: contributing a failsafe update meachanism for FIS from within ecos applications

AW: contributing a failsafe update meachanism for FIS from within ecos applications

[Neundorf, Alexander]

[-- 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 --]

Re: contributing a failsafe update meachanism for FIS from within ecos applications

[Andrew Lunn]

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

Hi Alex

I've been hacking you patch a bit. Here is my version. It compiles
cleanly, but i've not actually tried to run it. Please take a look and
let me know what you think.

        Andrew





[-- Attachment #2: redundant.diff --]
[-- Type: text/plain, Size: 11446 bytes --]

Index: redboot/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/ChangeLog,v
retrieving revision 1.213
diff -u -r1.213 ChangeLog
--- redboot/current/ChangeLog	13 Oct 2004 21:22:53 -0000	1.213
+++ redboot/current/ChangeLog	22 Oct 2004 19:12:13 -0000
@@ -1,3 +1,13 @@
+2004-10-22	Alexander Neundorf <Alexander.Neundorf@jenoptik.com>
+                Andrew Lunn        <andrew.lunn@ascom.ch>
+
+	* include/fis.h
+	* src/flash.c: Added basic support for a redundant FIS directory.
+	Currently there is no way to actually write a redundant directory,
+	but if such a directory where to exist, we can check if the
+	primary directory is valid and if not use the redundant one
+	instead.
+	
 2004-10-10	Iztok Zupet	 <iz@elsis.si>
 
 	* cdl/redboot.cdl: added CYGSEM_REDBOOT_DISK_IDE_VMWARE option.
Index: redboot/current/cdl/redboot.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/cdl/redboot.cdl,v
retrieving revision 1.68
diff -u -r1.68 redboot.cdl
--- redboot/current/cdl/redboot.cdl	13 Oct 2004 21:22:53 -0000	1.68
+++ redboot/current/cdl/redboot.cdl	22 Oct 2004 19:12:15 -0000
@@ -638,7 +638,33 @@
                       Negative block numbers count backwards from the last block.
                       eg 2 means block 2, -2 means the last but one block."
                 }
-    
+
+                cdl_component CYGOPT_REDBOOT_REDUNDANT_FIS {
+                    display         "Redundant Flash Image System Directory Support"
+                    default_value   0
+                    requires { 0 == CYGSEM_REDBOOT_FLASH_COMBINED_FIS_AND_CONFIG }
+                    description "
+                        This option enables the use of a redundant FIR 
+                        directory within RedBoot.  If enabled a flash block 
+                        will be reserved for a second copy of the fis 
+                        directory. Doing this allow for power failure safe 
+                        updates of the directory by the application."
+ 
+                    cdl_option CYGNUM_REDBOOT_FIS_REDUNDANT_DIRECTORY_BLOCK {
+                        display         "Flash block containing the backup Directory"
+                        flavor          data
+                        default_value   (-3)
+                        requires { CYGNUM_REDBOOT_FIS_REDUNDANT_DIRECTORY_BLOCK !=
+                                   CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK }    
+                        description "
+                           Which block of flash should hold the redundant 
+                           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"
                     active_if       { 0 != CYGNUM_REDBOOT_FLASH_RESERVED_BASE }
Index: redboot/current/include/fis.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/include/fis.h,v
retrieving revision 1.6
diff -u -r1.6 fis.h
--- redboot/current/include/fis.h	24 Aug 2002 11:20:55 -0000	1.6
+++ redboot/current/include/fis.h	22 Oct 2004 19:12:15 -0000
@@ -75,5 +75,22 @@
 
 struct fis_image_desc *fis_lookup(char *name, int *num);
 
+// These will need to move somewhere else when the fisfs needs to use
+// the following definitions.
+#ifdef CYGOPT_REDBOOT_REDUNDANT_FIS 
+#define CYG_REDBOOT_RFIS_VALID_MAGIC ".FisValid"  //exactly 10 bytes
+
+#define CYG_REDBOOT_RFIS_VALID       (0xa5a5)
+#define CYG_REDBOOT_RFIS_IN_PROGRESS (0xfdfd)
+#define CYG_REDBOOT_RFIS_EMPTY       (0xffff)
+
+struct fis_valid_info
+{
+   unsigned long version_count;
+   unsigned short valid_flag;
+};
+#endif // CYGOPT_REDBOOT_REDUNDANT_FIS 
+
 #endif // CYGOPT_REDBOOT_FIS
 #endif // _FIS_H_
+
Index: redboot/current/src/flash.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/src/flash.c,v
retrieving revision 1.69
diff -u -r1.69 flash.c
--- redboot/current/src/flash.c	1 Sep 2004 21:21:30 -0000	1.69
+++ redboot/current/src/flash.c	22 Oct 2004 19:12:17 -0000
@@ -169,6 +169,9 @@
 #ifdef CYGOPT_REDBOOT_FIS
 void *fis_work_block;
 void *fis_addr;
+#ifdef CYGOPT_REDBOOT_REDUNDANT_FIS
+void *redundant_fis_addr;
+#endif
 int fisdir_size;  // Size of FIS directory.
 #endif
 #ifdef CYGSEM_REDBOOT_FLASH_CONFIG
@@ -273,6 +276,31 @@
     fis_endian_fixup(fis_work_block);
 }
 
+#ifdef CYGOPT_REDBOOT_REDUNDANT_FIS
+void
+fis_erase_redundant_directory(void)
+{
+    int stat;
+    void *err_addr;
+
+#ifdef CYGSEM_REDBOOT_FLASH_LOCK_SPECIAL
+    // Ensure [quietly] that the directory is unlocked before trying
+    // to update
+    flash_unlock((void *)redundant_fis_addr, flash_block_size, 
+                 (void **)&err_addr);
+#endif
+    if ((stat = flash_erase(redundant_fis_addr, flash_block_size, 
+                            (void **)&err_addr)) != 0) {
+         diag_printf("Error erasing FIS directory at %p: %s\n", 
+                     err_addr, flash_errmsg(stat));
+    }
+#ifdef CYGSEM_REDBOOT_FLASH_LOCK_SPECIAL
+    // Ensure [quietly] that the directory is locked after the update
+    flash_lock((void *)fis_addr, flash_block_size, (void **)&err_addr);
+#endif
+}
+#endif
+
 static void
 fis_init(int argc, char *argv[])
 {
@@ -283,6 +311,9 @@
     struct option_info opts[1];
     CYG_ADDRESS redboot_flash_start;
     unsigned long redboot_image_size;
+#ifdef CYGOPT_REDBOOT_REDUNDANT_FIS
+    struct fis_valid_info fvi;
+#endif
 
     init_opts(&opts[0], 'f', false, OPTION_ARG_TYPE_FLG, 
               (void *)&full_init, (bool *)0, "full initialization, erases all of flash");
@@ -301,9 +332,23 @@
     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_REDUNDANT_FIS
+    CYG_ASSERT(sizeof(fvi)+sizeof(EFIS_MAGIC) <= sizeof(img->name),
+               "Size mismatch");
+    
+    //create the valid flag entry
+    memset(img, 0, sizeof(struct fis_image_desc));
+    strcpy(img->name, CYG_REDBOOT_RFIS_VALID_MAGIC);
+    fvi.valid_flag=CYG_REDBOOT_RFIS_VALID;
+    fvi.version_count=0;
+    memcpy(&img->name[sizeof(CYG_REDBOOT_RFIS_VALID_MAGIC)],&fvi,sizeof(fvi));
+    img++;
+#endif
+
+    // Create a pseudo image for RedBoot
 #ifdef CYGOPT_REDBOOT_FIS_RESERVED_BASE
     memset(img, 0, sizeof(*img));
     strcpy(img->name, "(reserved)");
@@ -361,6 +406,14 @@
     img->mem_base = (CYG_ADDRESS)fis_addr;
     img->size = fisdir_size;
     img++;
+#ifdef CYGOPT_REDBOOT_REDUNDANT_FIS
+    memset(img, 0, sizeof(*img));
+    strcpy(img->name, "Redundant FIS");
+    img->flash_base = (CYG_ADDRESS)redundant_fis_addr;
+    img->mem_base = (CYG_ADDRESS)redundant_fis_addr;
+    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
@@ -475,6 +528,9 @@
 #endif
     }
     fis_update_directory();
+#ifdef CYGOPT_REDBOOT_REDUNDANT_FIS
+    fis_erase_redundant_directory();
+#endif
 }
 
 static void
@@ -1355,7 +1411,13 @@
 do_flash_init(void)
 {
     int stat;
-
+#ifdef CYGOPT_REDBOOT_REDUNDANT_FIS
+    void *err_addr=0;
+    struct fis_valid_info fvi0;
+    struct fis_valid_info fvi1;
+    struct fis_image_desc img0;
+    struct fis_image_desc img1;
+#endif
     if (!__flash_init) {
         __flash_init = 1;
         if ((stat = flash_init(diag_printf)) != 0) {
@@ -1379,17 +1441,79 @@
         workspace_end = (unsigned char *)(workspace_end-fisdir_size);
         fis_work_block = workspace_end;
 # endif
+
         if (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK < 0) {
             fis_addr = (void *)((CYG_ADDRESS)flash_end + 1 +
                                 (CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK*flash_block_size));
         } else {
-            fis_addr = (void *)((CYG_ADDRESS)flash_start + 
+            fis_addr = (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) {
             diag_printf("FIS directory doesn't fit\n");
             return false;
         }
+
+#ifdef CYGOPT_REDBOOT_REDUNDANT_FIS
+        if (CYGNUM_REDBOOT_FIS_REDUNDANT_DIRECTORY_BLOCK < 0) {
+             redundant_fis_addr = 
+                  (void *)((CYG_ADDRESS)flash_end + 1 +
+                           (CYGNUM_REDBOOT_FIS_REDUNDANT_DIRECTORY_BLOCK *
+                            flash_block_size));
+        } else {
+             redundant_fis_addr = 
+                  (void *)((CYG_ADDRESS)flash_start +
+                           (CYGNUM_REDBOOT_FIS_REDUNDANT_DIRECTORY_BLOCK *
+                            flash_block_size));
+        }
+        
+        if (((CYG_ADDRESS)redundant_fis_addr + fisdir_size - 1) > 
+            (CYG_ADDRESS)flash_end) {
+             diag_printf("Redundant FIS directory doesn't fit\n");
+             return false;
+        }
+
+        // Read the first img from each directory and see if it
+        // contains the appropriate magic
+        FLASH_READ(fis_addr, &img0, sizeof(img0), (void **)&err_addr);
+        FLASH_READ(redundant_fis_addr, &img1, sizeof(img1), (void **)&err_addr);
+        
+        if (!strncmp(img0.name, CYG_REDBOOT_RFIS_VALID_MAGIC, 
+                     sizeof(CYG_REDBOOT_RFIS_VALID_MAGIC))) {
+             memcpy(&fvi0,&img0.name[sizeof(CYG_REDBOOT_RFIS_VALID_MAGIC)], 
+                    sizeof(fvi0));
+        } else {
+             memset(&fvi0, 0, sizeof(fvi0));
+        }
+        if (!strncmp(img1.name, CYG_REDBOOT_RFIS_VALID_MAGIC, 
+                     sizeof(CYG_REDBOOT_RFIS_VALID_MAGIC))) {
+             memcpy(&fvi1,&img1.name[sizeof(CYG_REDBOOT_RFIS_VALID_MAGIC)], 
+                    sizeof(fvi1));
+        } else {
+             memset(&fvi1, 0, sizeof(fvi1));
+        }
+        
+#ifdef REDBOOT_FLASH_REVERSE_BYTEORDER
+        fvi0.valid_flag = CYG_SWAP32(fvi0.valid_flag);
+        fvi0.version_count = CYG_SWAP32(fvi0.version_count);
+        fvi1.valid_flag = CYG_SWAP32(fvi1.valid_flag);
+        fvi1.version_count = CYG_SWAP32(fvi1.version_count);
+#endif
+        // Now check if the primary FIS directory is valid
+        if (fvi0.valid_flag != CYG_REDBOOT_RFIS_VALID) {
+             // Primary is not valid, try the secondary
+             if (fvi1.valid_flag == CYG_REDBOOT_RFIS_VALID) {
+                  // Valid, so swap primary and secondary
+                  void * tmp;
+                  tmp = fis_addr;
+                  fis_addr = redundant_fis_addr;
+                  redundant_fis_addr = tmp;
+             } else {
+                  // Both are invalid, stick with the invalid primary
+             }
+        }
+#endif
         fis_read_directory();
 #endif
     }

Re: contributing a failsafe update meachanism for FIS from within ecos applications

[Andrew Lunn]

On Fri, Oct 22, 2004 at 09:13:44PM +0200, Andrew Lunn wrote:
> Hi Alex
> 
> I've been hacking you patch a bit. Here is my version. It compiles
> cleanly, but i've not actually tried to run it. Please take a look and
> let me know what you think.

After a little thought i notices a bug. I do not check the version
when both FIS directories are valid. Easy to fix, i just don't have
the time at the moment.

        Andrew

Re: AW: contributing a failsafe update meachanism for FIS from within ecos applications

[Slawek]

Hello!
In message to "Andrew Lunn" <andrew@lunn.ch> sent Wed, 20 Oct 2004 18:11:18
+0200 you wrote:

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

Some additional ideas from somebody who watches the conversation:

1) Why do we need EFIS_IN_PROGRESS? Isn't EFIS_EMPTY enough? Both can't be
used to load the application anyway.

2) ".FisValid" suggest this is valid FIS entry while it doesn't need to be.
Why don't use separate name for valid and for invalid (empty or in progress)
fis tables? This could also save additional space used to mark
"valid/invalid" as this could be decided be the name.

-- 
Slawomir Piotrowski

Redboot lockups

[Curtis Whitley]

I am new to eCos and am trying to get setup so that I can write an eCos
application under Linux, to run on the ARM platform. For now, the platform
that I will be using does not exist, so I intend to test some parts of the
application on a PC (i386).

So, I built a Redboot image (i386) under Linux and made a floppy for the PC.
This image will boot fine and respond to either keyboard keys or to input
from the serial port, when running on an old machine (e.g. Gateway 200Mhz
Pentium).

When I run this image on my newer PC (3GHz Pentium 4), Redboot displays its
startup message and prompts, but then appears to lock up. It does not
respond to any keys or serial input.

Prior to this, I ran into problems trying to get a Redboot image running
under MS Virtual PC or VMWare on a single XP machine. At that time, I was
running eCos stuff under cygwin.

The bottom line is... I just want to write an application on my laptop (Dell
M60) under Linux and debug it. If I can do all that on the one laptop,
great. If not, I will attach a second PC via serial cable, but this seems to
be a problem.

I hear there is some kind of virtual machine for eCos under Linux. Is that
true?

I am running Redhat WS V3. Kernel is 2.4.21-15.EL on an i686.

Anyone with any ideas, please respond. Thanks!

Re: Redboot lockups

[Andrew Lunn]

> I hear there is some kind of virtual machine for eCos under Linux. Is that
> true?

Yes. Its the synthetic target.

http://ecos.sourceware.org/docs-latest/ref/hal-synth-arch.html

You will find this much easier to use than VMware, or a PC.

        Andrew

Re: contributing a failsafe update meachanism for FIS from within ecos applications

[Andrew Lunn]

> 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).

Maybe im missing something here....

You have three states:

Empty        11b
in_progress  01b
valid        00b

Since only one bit is ever changed you should always have a defined
state. The write either happened, or it did not. 

> 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.

640K is enought memory for any system.
4294967295 is enought IP addresses.
512 files in the root directory should be enough.

Why not just do it right and use modular arithmatic to handle wrap
around?

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 "

Setting the default as -1 is not a good idea. Thats the same as the
primary block. You probably want a requires statement that the
additional block is different from the primary block just to stop
people doing this....

+#define EFIS_MAGIC "$_FisValid_

Any particular reason for the $_ and _?

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

This does not look good. The version_count is in the same place as the
size. I expect uses are going to wonder why the size keeps
increasing. I would try to hide this inside the name.

Its also probably safer to use the existing struct fis_image_desc if
you can get it all inside the name. It means less trouble if somebody
comes along and adds a new field at the beginning etc.

+    memcpy(fvi->magic_name, EFIS_MAGIC, 12);

Using a memcpy on a string is not a good idea. Its much safer to do a
strcpy or strncpy.

+        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;

This looks ugly. I would put the memcmp (which should be strncpy())
inside get_valid_buf().

fis init should also probably erase the secondary fis directory just
to be safe.

        Andrew

Re: contributing a failsafe update meachanism for FIS from within ecos applications

[Andrew Lunn]

> 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. 

        Andrew

Re: contributing a failsafe update meachanism for FIS from within ecos applications

[Andrew Lunn]

> > 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.

But what about people who want to add this to an existing project and
there only free blocks in flash are somewhere else.

> > 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.

Not good. If its deleted how is it put back again? Do you have to fis
init so deleting all the existing entries? I hope not.

> 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 ?

It could happen very easily. Somebody might download the existing
redboot images from ecos.sourceware.org and use that. Somebody might
forget to enable this failsafe mode when they rebuild redboot and so
get an "old" image. There might be a redboot RAM image in FLASH which
does not understand the new entry...... If it can happen, it will
happen.

I would think about trying to use the existing format so that it
remains backward compatible. I think about add a dummy image to the
fis list. Dummy in that it start and length are 0. But the name would
be something like. "Fis Valid*" The sting is 16 charactors long. So
there are 5 unused charactors after the \0. You could put your magic
numbers there. This will probably be backward compatible with old
redboots. They will just see it as a normal entry with some junk after
the end of the string. Where i showed "*" in the sting, if you think
carefully about ASCII and the binary representation, you might be able
to come up with a set of charactors which differ by resetting 1's to
give some sort of meaningful mapping to the state of the FIS
directory.

        Andrew

Re: contributing a failsafe update meachanism for FIS from within ecos applications

[Andrew Lunn]

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. 

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

What happens when an old redboot reads the fis directory of a new
redboot with these valid/invalid entry? 

Does the entry show up on the fis list? Does it have a sensible name?
Is there any way to see if its valid/invalid etc.

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.

        Andrew

Re: contributing a failsafe update meachanism for FIS from within ecos applications

[Andrew Lunn]

> 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

Re: contributing a failsafe update meachanism for FIS from within ecos applications

[Andrew Lunn]

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.

        Andrew

contributing a failsafe update meachanism for FIS from within ecos applications

[Neundorf, Alexander]

[-- 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 --]