Flash bound checking fails if "upper (0xffffffff)" flash mapping
Jonathan Larmour
jifl@eCosCentric.com
Thu Apr 29 06:37:00 GMT 2004
sebastien Couret wrote:
> I have ran into a particular problem with the flash.
> It's about flash address bound checking.
> This problem only happens if the two following conditions are met :
>
> - Assert are activated (CYG_INFRA_DEBUG set)
> - Upper flash address is set to 0xffffffff
>
> What happens is that 'flash_hwr_init()' (which is implemented differently for
> each flash hardware) sets flash_info.end to zero (0xffffffff+1 !))
>
> As a result , in io/flash/current/src/flashiodev.c , following check fails !
> (flashend=0 and endpos>0 ), so flash access is always refused !
>
> #ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
> char *endpos = startpos + *len - 1;
> char *flashend = MIN( (char *)flash_info.end, dev->end);
> if ( startpos < dev->start )
> return -EINVAL;
> if ( endpos > flashend )
> return -EINVAL;
> #endif
>
> May be the better corrective will be to change the way of computing
> flash_info.end (by just substracting one) in each hardware flash driver.
>
> But it's quicker to change directly flashiodev.c.
> Here 's a patch :
>
> diff -a -w -r -u io/flash/current/src/v1.7/ io/flash/current/src/flashiodev.c
> --- io/flash/current/src/v1.7/flashiodev.c Thu Apr 22 11:31:55 2004
> +++ io/flash/current/src/flashiodev.c Mon Apr 19 18:47:35 2004
> @@ -126,7 +126,7 @@
>
> #ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
> char *endpos = startpos + *len - 1;
> - char *flashend = MIN( (char *)flash_info.end, dev->end);
> + char *flashend = MIN( (char *)flash_info.end - 1, dev->end);
[etc. similar]
Hi Sebastian,
I believe this patch just needs an extra adjustment. If you have a flash
device that ends at 0xffffffff and the flashiodev device also ends there,
then dev->end will also be 0xffffffff+1 i.e. 0. So the "if ( endpos >
flashend )" check will fail because flashend will have been set to 0.
Easily fixed anyway.
By the way, your mailer corrupted the patch - it was not whitespace clean,
and one line had been wrapped. In future it would be easier if you can
perhaps include the patch as an attachment. Thanks!
Jifl
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/ChangeLog,v
retrieving revision 1.35
diff -u -5 -p -r1.35 ChangeLog
--- ChangeLog 1 Dec 2003 14:33:58 -0000 1.35
+++ ChangeLog 29 Apr 2004 06:37:03 -0000
@@ -1,5 +1,11 @@
+2004-04-29 Sebastien Couret <sebastien.couret@elios-informatique.com>
+2004-04-29 Jonathan Larmour <jifl@eCosCentric.com>
+
+ * src/flashiodev.c: When checking flash upper bound, allow for end
+ of flash at 0xffffffff.
+
2003-11-27 David Woodhouse <dwmw2@infradead.org>
* src/flashiodev.c: Enable set_config() and implement
CYG_IO_SET_CONFIG_FLASH_FIS_NAME.
Index: src/flashiodev.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/src/flashiodev.c,v
retrieving revision 1.7
diff -u -5 -p -r1.7 flashiodev.c
--- src/flashiodev.c 1 Dec 2003 14:33:59 -0000 1.7
+++ src/flashiodev.c 29 Apr 2004 06:37:04 -0000
@@ -124,11 +124,11 @@ flashiodev_bread( cyg_io_handle_t handle
Cyg_ErrNo err = ENOERR;
#ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
char *endpos = startpos + *len - 1;
- char *flashend = MIN( (char *)flash_info.end, dev->end);
+ char *flashend = MIN( (char *)flash_info.end - 1, dev->end - 1);
if ( startpos < dev->start )
return -EINVAL;
if ( endpos > flashend )
return -EINVAL;
#endif
@@ -152,11 +152,11 @@ flashiodev_bwrite( cyg_io_handle_t handl
void *erraddr;
char *startpos = dev->start + pos;
#ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
char *endpos = startpos + *len - 1;
- char *flashend = MIN( (char *)flash_info.end, dev->end);
+ char *flashend = MIN( (char *)flash_info.end - 1, dev->end - 1);
if ( startpos < dev->start )
return -EINVAL;
if ( endpos > flashend )
return -EINVAL;
#endif
@@ -186,12 +186,12 @@ flashiodev_get_config( cyg_io_handle_t h
cyg_io_flash_getconfig_erase_t *e =
(cyg_io_flash_getconfig_erase_t *)buf;
char *startpos = dev->start + e->offset;
#ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
char *endpos = startpos + e->len - 1;
- char *flashend = MIN( (char *)flash_info.end, dev->end);
- if ( startpos < dev->start )
+ char *flashend = MIN( (char *)flash_info.end - 1, dev->end - 1);
+ if ( startpos < dev->start )
return -EINVAL;
if ( endpos > flashend )
return -EINVAL;
#endif
e->flasherr = flash_erase( startpos, e->len, e->err_address );
@@ -216,11 +216,11 @@ flashiodev_get_config( cyg_io_handle_t h
{
cyg_io_flash_getconfig_blocksize_t *b =
(cyg_io_flash_getconfig_blocksize_t *)buf;
#ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
char *startpos = dev->start + b->offset;
- char *flashend = MIN( (char *)flash_info.end, dev->end);
+ char *flashend = MIN( (char *)flash_info.end - 1, dev->end - 1);
if ( startpos < dev->start )
return -EINVAL;
if ( startpos > flashend )
return -EINVAL;
@@ -242,12 +242,14 @@ static Cyg_ErrNo
flashiodev_set_config( cyg_io_handle_t handle,
cyg_uint32 key,
const void* buf,
cyg_uint32* len)
{
+#ifdef CYGNUM_IO_FLASH_BLOCK_CFG_FIS_1
struct cyg_devtab_entry *tab = (struct cyg_devtab_entry *)handle;
struct flashiodev_priv_t *dev = (struct flashiodev_priv_t *)tab->priv;
+#endif
switch (key) {
#ifdef CYGNUM_IO_FLASH_BLOCK_CFG_FIS_1
case CYG_IO_SET_CONFIG_FLASH_FIS_NAME:
{
--
eCosCentric http://www.eCosCentric.com/ The eCos and RedBoot experts
--["No sense being pessimistic, it wouldn't work anyway"]-- Opinions==mine
More information about the Ecos-patches
mailing list