patch to add semihosting control for remote arm targets

patch to add semihosting control for remote arm targets

[Vadim Lebedev]

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


When debugging remote arm targets using  devices such as JEENI  from epitools 
the semihosting function slows execution by the target of SWI instruction 
nearly 100-fold.  This is especially annoying when you try to debug  linux 
kernel on the target as it uses SWI to implement system calls.  
The attached patch adds a set rdisemihosting {on/off/1/0/true/false} command 
to the gdb which allows one to enable or disable semihosting suppport.
the command shoud be executed before target rdi ....  command
The patch is against gdb 5.3 but i think will apply to older versions cleanly 
too.




Chang log entry

 Wed Mar 12 13:00:00 2003  Vadim Lebedev  (vadim at 7chips.com)

	* remote-rdi.c 		add set rdisemihosting command




[-- Attachment #2: gdb-rdi-semihosting.patch --]
[-- Type: text/x-diff, Size: 2174 bytes --]

--- remote-rdi.c.orig	Wed Mar 12 11:59:55 2003
+++ remote-rdi.c	Wed Mar 12 12:02:23 2003
@@ -118,6 +118,10 @@ static int rdi_heartbeat = 0;
 /* Target has ROM at address 0. */
 static int rom_at_zero = 0;
 
+
+/* target has semihosting enabled */
+static int rdi_semihosting = 1;
+
 /* Enable logging? */
 static int log_enable = 0;
 
@@ -319,10 +323,19 @@ device is attached to the remote system 
 
   rslt = angel_RDI_info (RDIVector_Catch, &arg1, &arg2);
   if (rslt != RDIError_NoError)
-    {
-      printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt));
-    }
+  {
+	  printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt));
+  }
+
+  arg1 = rdi_semihosting ? 1 : 0;
+  
+  rslt = angel_RDI_info(RDISemiHosting_SetState,  &arg1, &arg2);
+  if (rslt != RDIError_NoError)
+  {
+	  printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt));
+  }
 
+  
   arg1 = (unsigned long) "";
   rslt = angel_RDI_info (RDISet_Cmdline, &arg1, &arg2);
   if (rslt != RDIError_NoError)
@@ -1051,14 +1064,24 @@ _initialize_remote_rdi (void)
      &setlist, &showlist);
 
   add_setshow_boolean_cmd
-    ("rdiheartbeat", no_class, &rdi_heartbeat,
-     "Set enable for ADP heartbeat packets.\n"
-     "I don't know why you would want this. If you enable them,\n"
-     "it will confuse ARM and EPI JTAG interface boxes as well\n"
-     "as the Angel Monitor.\n",
-     "Show enable for ADP heartbeat packets.\n",
-     NULL, NULL,
-     &setlist, &showlist);
+		  ("rdiheartbeat", no_class, &rdi_heartbeat,
+		   "Set enable for ADP heartbeat packets.\n"
+		   "I don't know why you would want this. If you enable them,\n"
+		   "it will confuse ARM and EPI JTAG interface boxes as well\n"
+		   "as the Angel Monitor.\n",
+		   "Show enable for ADP heartbeat packets.\n",
+		   NULL, NULL,
+		   &setlist, &showlist);
+
+  add_setshow_boolean_cmd
+		  ("rdisemihosting", no_class, &rdi_semihosting,
+		   "Set semihosting support.\n"
+		   "A true value activates semihosting false value deactivates it.\n",
+		   "Show enable for semihosting.\n",
+		   NULL, NULL,
+		   &setlist, &showlist);
+
+  
 }
 
 /* A little dummy to make linking with the library succeed. */

Re: patch to add semihosting control for remote arm targets

[Richard Earnshaw]

> 
> When debugging remote arm targets using  devices such as JEENI  from epitools 
> the semihosting function slows execution by the target of SWI instruction 
> nearly 100-fold.  This is especially annoying when you try to debug  linux 
> kernel on the target as it uses SWI to implement system calls.  
> The attached patch adds a set rdisemihosting {on/off/1/0/true/false} command 
> to the gdb which allows one to enable or disable semihosting suppport.
> the command shoud be executed before target rdi ....  command
> The patch is against gdb 5.3 but i think will apply to older versions cleanly 
> too.
> 
> 

The idea is a good one, but there are several problems with this patch.

1) The formatting does not conform to GNU coding standards.  (use 
gdb_indent.sh)
2) I think we should have a "set rdi" command for the two rdi features (in 
the same way that we have set debug).  Heartbeat and semihosting should be 
sub-variables of that command.
3) Ideally, it should be possible to turn off semihosting after we have 
connected to the target.

Do you have a copyright assignment on file?

Other comments are interspersed below.

>  Wed Mar 12 13:00:00 2003  Vadim Lebedev  (vadim at 7chips.com)
> 
> 	* remote-rdi.c 		add set rdisemihosting command
> 

Formatting.  Please follow ChangeLog conventions.


> +/* target has semihosting enabled */

Formatting.  Capital letter at the start.  Full stop and two spaces at the 
end before the close-comment.

>    if (rslt != RDIError_NoError)
> -    {
> -      printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt));
> -    }
> +  {
> +	  printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt));
> +  }

Indentation -- why have you changed it?

> +
> +  arg1 = rdi_semihosting ? 1 : 0;
> +  
> +  rslt = angel_RDI_info(RDISemiHosting_SetState,  &arg1, &arg2);
> +  if (rslt != RDIError_NoError)
> +  {
> +	  printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt));
> +  }

Indentation.

>  
> +  
>    arg1 = (unsigned long) "";
>    rslt = angel_RDI_info (RDISet_Cmdline, &arg1, &arg2);
>    if (rslt != RDIError_NoError)
> @@ -1051,14 +1064,24 @@ _initialize_remote_rdi (void)
>       &setlist, &showlist);
>  
>    add_setshow_boolean_cmd
> -    ("rdiheartbeat", no_class, &rdi_heartbeat,
> -     "Set enable for ADP heartbeat packets.\n"
> -     "I don't know why you would want this. If you enable them,\n"
> -     "it will confuse ARM and EPI JTAG interface boxes as well\n"
> -     "as the Angel Monitor.\n",
> -     "Show enable for ADP heartbeat packets.\n",
> -     NULL, NULL,
> -     &setlist, &showlist);
> +		  ("rdiheartbeat", no_class, &rdi_heartbeat,
> +		   "Set enable for ADP heartbeat packets.\n"
> +		   "I don't know why you would want this. If you enable them,\n"
> +		   "it will confuse ARM and EPI JTAG interface boxes as well\n"
> +		   "as the Angel Monitor.\n",
> +		   "Show enable for ADP heartbeat packets.\n",
> +		   NULL, NULL,
> +		   &setlist, &showlist);

Indentation.

> +
> +  add_setshow_boolean_cmd
> +		  ("rdisemihosting", no_class, &rdi_semihosting,
> +		   "Set semihosting support.\n"
> +		   "A true value activates semihosting false value deactivates it.\n",

Unnecessary repetition.  "A true value activates semihosting.\n" is 
sufficient.

> +		   "Show enable for semihosting.\n",
> +		   NULL, NULL,
> +		   &setlist, &showlist);
> +
> +  

Indetation -- delete unnecessary blank lines.

R.