public inbox for ecos-devel@sourceware.org
 help / color / mirror / Atom feed
* Redboot load command is crippled
@ 2006-11-19 12:46 John Clark
  2006-11-19 16:01 ` Andrew Lunn
  0 siblings, 1 reply; 5+ messages in thread
From: John Clark @ 2006-11-19 12:46 UTC (permalink / raw)
  To: ecos-devel; +Cc: asl

Hello,

I've noticed that the Redboot load command is crippled whenever 
"Validate RAM addresses during load" 
(CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS) is disabled.  It looks like 
when the load into flash option was added to load.c, a call to 
valid_address() was added to load_elf_image() and load_srec_image() to 
determine whether to write to flash or not.

I'm new to using Redboot and eCos.  I've been playing with it on an i386 
PC target and loading eCos applications into extended memory above 1 
MB.  Redboot seems to require address validation to be disabled because 
this platform doesn't implement cyg_plf_memory_segment().

Anyhow, because of the unconditional calls to valid_address() now, load 
reports success but fails to actually write to memory outside of its 
known range.  For a quick fix, I put a check for 
CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS around the calls to 
valid_address().  Unfortunately, that breaks the load into flash feature 
completely when address validation is disabled.

Everything else looks great.  I'm really starting to like eCos.  Thanks.

- John

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Redboot load command is crippled
  2006-11-19 12:46 Redboot load command is crippled John Clark
@ 2006-11-19 16:01 ` Andrew Lunn
  2006-11-19 22:51   ` John Clark
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2006-11-19 16:01 UTC (permalink / raw)
  To: John Clark; +Cc: ecos-devel, asl

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

On Sun, Nov 19, 2006 at 07:45:54AM -0500, John Clark wrote:
> Hello,
> 
> Anyhow, because of the unconditional calls to valid_address() now, load 
> reports success but fails to actually write to memory outside of its 
> known range.  For a quick fix, I put a check for 
> CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS around the calls to 
> valid_address().  Unfortunately, that breaks the load into flash feature 
> completely when address validation is disabled.

Hi John.

How does it break the load?  Please could you try the attached patch.

    Thanks

        Andrew

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

Index: redboot/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/ChangeLog,v
retrieving revision 1.249
diff -u -r1.249 ChangeLog
--- redboot/current/ChangeLog	6 Sep 2006 14:26:52 -0000	1.249
+++ redboot/current/ChangeLog	19 Nov 2006 16:00:19 -0000
@@ -1,3 +1,8 @@
+2006-11-19  Andrew Lunn  <lunn@laptop.lunn.ch>
+
+	* src/load.c: Only call valid_address() if
+	CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS is enabled.
+
 2006-09-06  Andrew Lunn  <andrew.lunn@ascom.ch>
 
 	* cdl/redboot.cdl: Fix description of CYGSEM_REDBOOT_DISK_IDE.
Index: redboot/current/src/load.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/src/load.c,v
retrieving revision 1.47
diff -u -r1.47 load.c
--- redboot/current/src/load.c	20 Jul 2006 20:27:47 -0000	1.47
+++ redboot/current/src/load.c	19 Nov 2006 16:00:20 -0000
@@ -422,9 +422,11 @@
                     redboot_getc_terminate(true);
                     return 0;
                 }
-                if (valid_address(addr)) {
+#ifdef CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS
+                if (valid_address(addr)) 
+#endif
                   *addr++ = ch;
-                }
+                
 #ifdef CYGBLD_REDBOOT_LOAD_INTO_FLASH
                 else {
                   flash_load_write(addr, ch);
@@ -567,9 +569,11 @@
             offset += count;
             while (count-- > 0) {
                 val = _hex2(getc, 1, &sum);
-                if (valid_address(addr)) {
+#ifdef CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS
+                if (valid_address(addr)) 
+#endif
                   *addr++ = val;
-                }
+                
 #ifdef CYGBLD_REDBOOT_LOAD_INTO_FLASH
                 else {
                   flash_load_write(addr, val);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Redboot load command is crippled
  2006-11-19 16:01 ` Andrew Lunn
@ 2006-11-19 22:51   ` John Clark
  2006-11-20 23:13     ` Andrew Lunn
  0 siblings, 1 reply; 5+ messages in thread
From: John Clark @ 2006-11-19 22:51 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-devel

Hello Andrew,

Your patch matches what I have already done to get it to work for me.  
Without that change, it would never write anything to memory through the 
addr pointer if the address was outside of the memory range known to 
Redboot on the PC platform (beyond the first 640K).

My only concern now is that the "Allow the load-command write into 
Flash" (CYGBLD_REDBOOT_LOAD_INTO_FLASH) option requires that "Validate 
RAM addresses during load" (CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS) 
option be enabled as well.  Otherwise, the code assuming that an invalid 
RAM address is a flash address just won't work.

- John

Andrew Lunn wrote:
> Hi John.
>
> How does it break the load?  Please could you try the attached patch.
>
>     Thanks
>
>         Andrew
>   

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Redboot load command is crippled
  2006-11-19 22:51   ` John Clark
@ 2006-11-20 23:13     ` Andrew Lunn
  2006-11-22  6:51       ` John Clark
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2006-11-20 23:13 UTC (permalink / raw)
  To: John Clark; +Cc: ecos-devel

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

On Sun, Nov 19, 2006 at 05:51:08PM -0500, John Clark wrote:
> Hello Andrew,
> 
> Your patch matches what I have already done to get it to work for me.  
> Without that change, it would never write anything to memory through the 
> addr pointer if the address was outside of the memory range known to 
> Redboot on the PC platform (beyond the first 640K).
> 
> My only concern now is that the "Allow the load-command write into 
> Flash" (CYGBLD_REDBOOT_LOAD_INTO_FLASH) option requires that "Validate 
> RAM addresses during load" (CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS) 
> option be enabled as well.  Otherwise, the code assuming that an invalid 
> RAM address is a flash address just won't work.

I see what you mean. How about this patch.

  Andrew

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

Index: redboot/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/ChangeLog,v
retrieving revision 1.249
diff -u -r1.249 ChangeLog
--- redboot/current/ChangeLog	6 Sep 2006 14:26:52 -0000	1.249
+++ redboot/current/ChangeLog	20 Nov 2006 23:11:49 -0000
@@ -1,3 +1,8 @@
+2006-11-19  Andrew Lunn  <lunn@laptop.lunn.ch>
+
+	* src/load.c: Only call valid_address() if
+	CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS is enabled.
+
 2006-09-06  Andrew Lunn  <andrew.lunn@ascom.ch>
 
 	* cdl/redboot.cdl: Fix description of CYGSEM_REDBOOT_DISK_IDE.
Index: redboot/current/cdl/redboot.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/cdl/redboot.cdl,v
retrieving revision 1.76
diff -u -r1.76 redboot.cdl
--- redboot/current/cdl/redboot.cdl	6 Sep 2006 14:26:53 -0000	1.76
+++ redboot/current/cdl/redboot.cdl	20 Nov 2006 23:11:51 -0000
@@ -240,10 +240,13 @@
         cdl_option CYGBLD_REDBOOT_LOAD_INTO_FLASH {
             display       "Allow the load-command write into Flash."
             default_value 0
-            active_if     CYGPKG_REDBOOT_FLASH     
+            active_if     CYGPKG_REDBOOT_FLASH
+	    requires	  CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS
             compile       flash_load.c
             description "
-                Write images direct to Flash via the load command."
+                Write images direct to Flash via the load command.
+                We assume anthing which is invalid RAM is flash, hence
+                the requires statement"
         }
         cdl_option CYGBLD_BUILD_REDBOOT_WITH_CKSUM {
             display       "Include POSIX checksum command"
Index: redboot/current/src/load.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/redboot/current/src/load.c,v
retrieving revision 1.47
diff -u -r1.47 load.c
--- redboot/current/src/load.c	20 Jul 2006 20:27:47 -0000	1.47
+++ redboot/current/src/load.c	20 Nov 2006 23:11:52 -0000
@@ -422,9 +422,11 @@
                     redboot_getc_terminate(true);
                     return 0;
                 }
-                if (valid_address(addr)) {
+#ifdef CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS
+                if (valid_address(addr)) 
+#endif
                   *addr++ = ch;
-                }
+                
 #ifdef CYGBLD_REDBOOT_LOAD_INTO_FLASH
                 else {
                   flash_load_write(addr, ch);
@@ -567,9 +569,11 @@
             offset += count;
             while (count-- > 0) {
                 val = _hex2(getc, 1, &sum);
-                if (valid_address(addr)) {
+#ifdef CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS
+                if (valid_address(addr)) 
+#endif
                   *addr++ = val;
-                }
+                
 #ifdef CYGBLD_REDBOOT_LOAD_INTO_FLASH
                 else {
                   flash_load_write(addr, val);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Redboot load command is crippled
  2006-11-20 23:13     ` Andrew Lunn
@ 2006-11-22  6:51       ` John Clark
  0 siblings, 0 replies; 5+ messages in thread
From: John Clark @ 2006-11-22  6:51 UTC (permalink / raw)
  To: ecos-devel

Hello Andrew,

That patch works for me.  There is one minor error.  You misspelled
"anything" as "anthing" in the modified description text in redboot.cdl.

Thank you!

- John

Andrew Lunn wrote:
> I see what you mean. How about this patch.
>
>   Andrew
>   


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2006-11-22  6:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-11-19 12:46 Redboot load command is crippled John Clark
2006-11-19 16:01 ` Andrew Lunn
2006-11-19 22:51   ` John Clark
2006-11-20 23:13     ` Andrew Lunn
2006-11-22  6:51       ` John Clark

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