[Patch] Extended flash support

[Mike Wrighton <mike_wrighton at mentor dot com>]

Hi,

I have a patch which extends the current flash support for embedded 
targets in the following ways:

1. A new command, "flash-erase" has been added which erases all regions 
of flash memory.
2. A -verify option has been added to the load command which enables 
post-load verification (this applies to both flash and non-flash memory 
regions).

I have a question regarding testcases of these features, as both parts 
are less than straightforward to verify. Part 1 would require creating a 
dummy target with some flash memory regions and adding some support for 
the flash packets (e.g. vFlashErase) in gdbserver. Part 2 would probably 
require creating a target which returns corrupted data after the load, 
forcing a verify error. Are these testcases required, and if so, are 
there any existing examples for testing features that are currently 
unimplemented in gdbserver?

Thanks,
Mike


Index: gdb/symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.351
diff -u -r1.351 symfile.c
--- gdb/symfile.c	9 Nov 2012 19:58:01 -0000	1.351
+++ gdb/symfile.c	22 Nov 2012 15:38:10 -0000
@@ -2068,6 +2068,8 @@
   struct load_section_data cbdata;
   struct load_progress_data total_progress;
   struct ui_out *uiout = current_uiout;
+  int arg_idx;
+  int verify = 0;
 
   CORE_ADDR entry;
   char **argv;
@@ -2087,19 +2089,29 @@
   filename = tilde_expand (argv[0]);
   make_cleanup (xfree, filename);
 
-  if (argv[1] != NULL)
+  arg_idx = 1;
+  if (argv[arg_idx] != NULL)
     {
       char *endptr;
 
-      cbdata.load_offset = strtoul (argv[1], &endptr, 0);
-
-      /* If the last word was not a valid number then
-         treat it as a file name with spaces in.  */
-      if (argv[1] == endptr)
-        error (_("Invalid download offset:%s."), argv[1]);
-
-      if (argv[2] != NULL)
-	error (_("Too many parameters."));
+      if (strcmp (argv[arg_idx], "-verify") == 0)
+        {
+          verify = 1;
+          arg_idx++;
+        }
+
+      if (argv[arg_idx] != NULL)
+        {
+          cbdata.load_offset = strtoul (argv[arg_idx], &endptr, 0);
+
+          /* If the last word was not a valid number then
+             treat it as a file name with spaces in.  */
+          if (argv[arg_idx] == endptr)
+            error (_("Invalid download offset:%s."), argv[arg_idx]);
+    
+          if (argv[arg_idx + 1] != NULL)
+    	    error (_("Too many parameters."));
+        }
     }
 
   /* Open the file for loading.  */
@@ -2126,7 +2138,7 @@
   gettimeofday (&start_time, NULL);
 
   if (target_write_memory_blocks (cbdata.requests, flash_discard,
-				  load_progress) != 0)
+				  load_progress, verify) != 0)
     error (_("Load failed"));
 
   gettimeofday (&end_time, NULL);
Index: gdb/target-memory.c
===================================================================
RCS file: /cvs/src/src/gdb/target-memory.c,v
retrieving revision 1.14
diff -u -r1.14 target-memory.c
--- gdb/target-memory.c	4 Jan 2012 08:17:13 -0000	1.14
+++ gdb/target-memory.c	22 Nov 2012 15:38:10 -0000
@@ -22,6 +22,8 @@
 #include "vec.h"
 #include "target.h"
 #include "memory-map.h"
+#include "memattr.h"
+#include "ui-out.h"
 
 #include "gdb_assert.h"
 
@@ -310,12 +312,13 @@
 int
 target_write_memory_blocks (VEC(memory_write_request_s) *requests,
 			    enum flash_preserve_mode preserve_flash_p,
-			    void (*progress_cb) (ULONGEST, void *))
+			    void (*progress_cb) (ULONGEST, void *),
+			    int verify)
 {
   struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
   VEC(memory_write_request_s) *blocks = VEC_copy (memory_write_request_s,
 						  requests);
-  unsigned i;
+  unsigned i, j;
   int err = 0;
   struct memory_write_request *r;
   VEC(memory_write_request_s) *regular = NULL;
@@ -433,6 +436,28 @@
       target_flash_done ();
     }
 
+  if (verify)
+    {
+      for (i = 0; VEC_iterate (memory_write_request_s, blocks, i, r); ++i)
+        {
+          gdb_byte *read_data = xmalloc (r->end - r->begin);
+
+          if (target_read_memory (r->begin, read_data, r->end - r->begin) != 0)
+            error ("Download verification failed, could not read memory region 0x%x - 0x%x", 
+                   (unsigned int) r->begin, (unsigned int) r->end);
+
+          for (j = 0; j < r->end - r->begin; j++)
+            if (read_data[j] != r->data[j])
+              error ("Load verification failed at address 0x%x",
+                     (unsigned int) r->begin + j);
+
+          ui_out_message (current_uiout, 0, "Verified region, address 0x%x size 0x%x\n",
+                          (unsigned int) r->begin, (unsigned int) (r->end - r->begin));
+
+          xfree(read_data);
+        }
+    }
+
  out:
   do_cleanups (back_to);
 
Index: gdb/target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.316
diff -u -r1.316 target.c
--- gdb/target.c	9 Nov 2012 19:58:01 -0000	1.316
+++ gdb/target.c	22 Nov 2012 15:38:12 -0000
@@ -4668,6 +4668,40 @@
   target_rcmd (cmd, gdb_stdtarg);
 }
 
+void
+flash_erase_all_command (char *cmd, int from_tty)
+{
+  int i;
+  int found_flash_region = 0;
+  struct mem_region *m;
+  struct mem_attrib *attrib;
+  struct cleanup *cleanup_tuple;
+
+  VEC(mem_region_s) * mem_regions = target_memory_map ();
+
+  for (i = 0; VEC_iterate (mem_region_s, mem_regions, i, m); i++)
+    {
+      attrib = &m->attrib;
+      if (attrib->mode == MEM_FLASH)
+        {
+          found_flash_region = 1;
+          target_flash_erase (m->lo, m->hi - m->lo);
+          cleanup_tuple = make_cleanup_ui_out_tuple_begin_end (current_uiout, "erasing-regions");
+          ui_out_message (current_uiout, 0, "Erasing flash region at address ");
+          ui_out_field_fmt (current_uiout, "address", "0x%x", (unsigned int) m->lo);
+          ui_out_message (current_uiout, 0, ", size = ");
+          ui_out_field_fmt (current_uiout, "size", "%lu", (unsigned int) m->hi - m->lo);
+          ui_out_message (current_uiout, 0, "\n");
+          do_cleanups (cleanup_tuple);
+        }
+    }
+
+  if (found_flash_region)
+    target_flash_done ();
+  else
+    ui_out_message (current_uiout, 0, "No flash regions found.");
+}
+
 /* Print the name of each layers of our target stack.  */
 
 static void
@@ -4881,6 +4915,8 @@
 			   set_target_permissions, NULL,
 			   &setlist, &showlist);
 
+  add_com ("flash-erase", no_class, flash_erase_all_command,
+           _("Erase all flash memory blocks."));
 
   target_dcache = dcache_init ();
 }
Index: gdb/target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.244
diff -u -r1.244 target.h
--- gdb/target.h	9 Nov 2012 19:58:01 -0000	1.244
+++ gdb/target.h	22 Nov 2012 15:38:13 -0000
@@ -1032,6 +1032,9 @@
    is returned.  */
 VEC(mem_region_s) *target_memory_map (void);
 
+/* Command to erase all flash regions on the target.  */
+void flash_erase_all_command (char *cmd, int from_tty);
+
 /* Erase the specified flash region.  */
 void target_flash_erase (ULONGEST address, LONGEST length);
 
@@ -1075,11 +1078,13 @@
      feedback to user.  It will be called with the baton corresponding
      to the request currently being written.  It may also be called
      with a NULL baton, when preserved flash sectors are being rewritten.
+   VERIFY_FLASH indicates whether to perform verification on flash data.
 
    The function returns 0 on success, and error otherwise.  */
 int target_write_memory_blocks (VEC(memory_write_request_s) *requests,
 				enum flash_preserve_mode preserve_flash_p,
-				void (*progress_cb) (ULONGEST, void *));
+				void (*progress_cb) (ULONGEST, void *),
+				int verify);
 
 /* Print a line about the current target.  */
 
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.1025
diff -u -r1.1025 gdb.texinfo
--- gdb/doc/gdb.texinfo	15 Nov 2012 21:25:43 -0000	1.1025
+++ gdb/doc/gdb.texinfo	22 Nov 2012 15:38:29 -0000
@@ -17192,7 +17192,7 @@
 @table @code
 
 @kindex load @var{filename}
-@item load @var{filename}
+@item load @var{filename} [-verify]
 @anchor{load}
 Depending on what remote debugging facilities are configured into
 @value{GDBN}, the @code{load} command may be available.  Where it exists, it
@@ -17214,9 +17214,22 @@
 Depending on the remote side capabilities, @value{GDBN} may be able to
 load programs into flash memory.
 
+The optional @code{-verify} flag enables verification of the data that was 
+loaded.
+
 @code{load} does not repeat if you press @key{RET} again after using it.
 @end table
 
+@table @code
+
+@kindex flash-erase
+@item flash-erase
+@anchor{flash-erase}
+
+Erases all known flash memory regions on the target.
+
+@end table
+
 @node Byte Order
 @section Choosing Target Byte Order
 
@@ -32522,6 +32535,27 @@
 @subsubheading Example
 N.A.
 
+@subheading The @code{-target-flash-erase} Command
+@findex -target-flash-erase
+
+@subsubheading Synopsis
+
+@smallexample
+ -target-flash-erase
+@end smallexample
+
+Erases all known flash memory regions on the target.
+
+The corresponding @value{GDBN} command is @samp{flash-erase}.
+
+The output is a list of flash regions which were erased, by address/size.
+
+@smallexample
+(gdb)
+-target-flash-erase
+^done,erased-regions=@{address="0x0",size="262144"@}
+(gdb)
+@end smallexample
 
 @subheading The @code{-target-select} Command
 @findex -target-select
Index: gdb/mi/mi-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.c,v
retrieving revision 1.64
diff -u -r1.64 mi-cmds.c
--- gdb/mi/mi-cmds.c	17 Oct 2012 00:53:23 -0000	1.64
+++ gdb/mi/mi-cmds.c	22 Nov 2012 15:38:29 -0000
@@ -134,6 +134,7 @@
   DEF_MI_CMD_MI ("target-file-delete", mi_cmd_target_file_delete),
   DEF_MI_CMD_MI ("target-file-get", mi_cmd_target_file_get),
   DEF_MI_CMD_MI ("target-file-put", mi_cmd_target_file_put),
+  DEF_MI_CMD_MI ("target-flash-erase", mi_cmd_target_flash_erase),
   DEF_MI_CMD_CLI ("target-select", "target", 1),
   DEF_MI_CMD_MI ("thread-info", mi_cmd_thread_info),
   DEF_MI_CMD_MI ("thread-list-ids", mi_cmd_thread_list_ids),
Index: gdb/mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.56
diff -u -r1.56 mi-cmds.h
--- gdb/mi/mi-cmds.h	31 Aug 2012 08:41:57 -0000	1.56
+++ gdb/mi/mi-cmds.h	22 Nov 2012 15:38:29 -0000
@@ -91,6 +91,7 @@
 extern mi_cmd_argv_ftype mi_cmd_target_file_get;
 extern mi_cmd_argv_ftype mi_cmd_target_file_put;
 extern mi_cmd_argv_ftype mi_cmd_target_file_delete;
+extern mi_cmd_argv_ftype mi_cmd_target_flash_erase;
 extern mi_cmd_argv_ftype mi_cmd_thread_info;
 extern mi_cmd_argv_ftype mi_cmd_thread_list_ids;
 extern mi_cmd_argv_ftype mi_cmd_thread_select;
Index: gdb/mi/mi-main.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
retrieving revision 1.224
diff -u -r1.224 mi-main.c
--- gdb/mi/mi-main.c	13 Nov 2012 21:19:10 -0000	1.224
+++ gdb/mi/mi-main.c	22 Nov 2012 15:38:30 -0000
@@ -463,6 +463,11 @@
   detach_command (NULL, 0);
 }
 
+void mi_cmd_target_flash_erase (char *command, char **argv, int argc)
+{
+  flash_erase_all_command (NULL, 0);
+}
+
 void
 mi_cmd_thread_select (char *command, char **argv, int argc)
 {