Hi all, The attached patch "flash_probe.patch" moves the invocation of a flash device size probe[1] in the powerpc flash driver framework from argcheck() to bankValidate(). Previously, the latter was called by the former before the probe had happened and bankValidate()s subsequent call of BSP_flashCheckId() caused it to access memory that is potentially unmapped.
On my MVME3100, this previous behaviour caused a machine check exception. [1] In my ever-humble opinion, this whole probe business is a bad idea from the get-go... The attached diff "flash_don't_probe.patch" - which I'm not suggesting be committed, but *is* what I'm using - reworks it on the MVME3100 so that the board's configuration is read from the "Vital Product Data" EEPROM, giving the flash address and size. I've had trouble with the VPD routines, however (and haven't fully debugged that)... The attached "vpd.patch" seems to have sorted that or me in the interim. I'd be interested in hearing others' thoughts on this :-) -- Nick Withers Embedded Systems Programmer Department of Nuclear Physics, Research School of Physics and Engineering The Australian National University (CRICOS: 00120C)
diff --git a/c/src/lib/libbsp/powerpc/mvme3100/flash/flashcfg.c b/c/src/lib/libbsp/powerpc/mvme3100/flash/flashcfg.c index 106d5c2..baca9a8 100644 --- a/c/src/lib/libbsp/powerpc/mvme3100/flash/flashcfg.c +++ b/c/src/lib/libbsp/powerpc/mvme3100/flash/flashcfg.c @@ -48,6 +48,7 @@ #include <rtems.h> #include <bsp.h> #include <libcpu/spr.h> +#include <bsp/vpd.h> #include <stdio.h> #include <bsp/flashPgmPvt.h> @@ -57,11 +58,8 @@ SPR_RO(TBRL) #define STATIC static static struct bankdesc mvme3100Flash[] = { - /* - * Bank is populated from the top; make max_size negative to - * indicate this - */ - { 0xf8000000, 0, - 0x08000000, 2, BSP_flash_vendor_spansion, 0, 0, 0 }, + /* bankcheck() will probe the board's VPD to obtain flash address and size */ + { 0, 0, 0, 2, BSP_flash_vendor_spansion, 0, 0, 0 } }; STATIC struct bankdesc * @@ -70,8 +68,35 @@ bankcheck(int bank, int quiet) if ( bank ) { if ( !quiet ) fprintf(stderr,"Invalid flash bank #%i\n",bank); - return 0; + return NULL; } + + if ( !mvme3100Flash[bank].start ) { +const size_t flash_vpd_bytes = 12; /* The size in bytes of VPD data for each Flash Device Configuration entry; arguably belongs in vpd.h */ +const ptrdiff_t flash_vpd_offset_size = 11; /* The byte offset within a VPD Flash Device Configuration entry to the encoded size field */ + +unsigned char vpd_data[flash_vpd_bytes]; + + if ( BSP_vpdRetrieveKey(FlashConfig,vpd_data,sizeof(vpd_data),bank)!=sizeof(vpd_data) ) { + if ( !quiet ) + fprintf(stderr,"Failed reading flash VPD\n"); + return NULL; + } + else { +unsigned char size = *(vpd_data+flash_vpd_offset_size); + + if (size && size <= sizeof (mvme3100Flash[bank].max_size)*CHAR_BIT) { + mvme3100Flash[bank].max_size = (512*1024)*(1<<(size-1)); + mvme3100Flash[bank].start = 0xFFFFFFFF-(mvme3100Flash[bank].max_size-1); + } + else { + if ( !quiet ) + fprintf(stderr,"Invalid flash VPD size encoding 0x%hhx\n",size); + return NULL; + } + } + } + return &mvme3100Flash[bank]; } @@ -110,7 +135,7 @@ uint32_t mhz = BSP_bus_frequency/BSP_time_base_divisor/1000; /* BSP ops (detect banks, handle write-protection on board) */ struct flash_bsp_ops BSP_flashBspOps = { - bankcheck: bankcheck, - flash_wp: flash_wp, + bankcheck: bankcheck, + flash_wp: flash_wp, read_us_timer: read_us_timer, };
>From 8b99fe9709ca5aaadb22b81cd037a9521f4de54b Mon Sep 17 00:00:00 2001 From: Nick Withers <nick.with...@anu.edu.au> Date: Thu, 30 Jan 2014 12:58:41 +1100 Subject: [PATCH] Move the flash size probe into bankValidate() Previously, bankValidate() could be called (e.g., BSP_flashWrite() -> regionCheckAndErase() -> argcheck() -> bankValidate()) without the probe having happened. When it then invoked BSP_flashCheckId(), unmapped memory could be read, possibly causing a fatal exception. --- c/src/lib/libbsp/powerpc/shared/flash/flash.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/c/src/lib/libbsp/powerpc/shared/flash/flash.c b/c/src/lib/libbsp/powerpc/shared/flash/flash.c index 0037fe7..b7f1678 100644 --- a/c/src/lib/libbsp/powerpc/shared/flash/flash.c +++ b/c/src/lib/libbsp/powerpc/shared/flash/flash.c @@ -456,6 +456,11 @@ struct bankdesc *b = BSP_flashBspOps.bankcheck(bank, quiet); return 0; } + if ( !b->size && !(b->size = BSP_flashProbeSize(b)) ) { + fprintf(stderr,"Configuration Error - unable to determine flash size\n"); + return 0; + } + if ( !b->dd && !(b->dd = BSP_flashCheckId(b, b->start,1)) ) { fprintf(stderr,"Error: unable to detect flash device in bank #%i\n", bank); return 0; @@ -485,10 +490,6 @@ struct bankdesc *b; return 0; } - if ( !b->size && !(b->size = BSP_flashProbeSize(b)) ) { - fprintf(stderr,"Configuration Error - unable to determine flash size\n"); - return 0; - } if ( offset + size > b->size ) { fprintf(stderr,"Error: requested size exceeds available flash (0x%08"PRIx32" bytes)\n", b->size); return 0; -- 1.8.5.2
diff --git a/c/src/lib/libbsp/powerpc/shared/motorola/vpd.c b/c/src/lib/libbsp/powerpc/shared/motorola/vpd.c index 4b0bbef..1cbfebc 100644 --- a/c/src/lib/libbsp/powerpc/shared/motorola/vpd.c +++ b/c/src/lib/libbsp/powerpc/shared/motorola/vpd.c @@ -137,7 +137,7 @@ static int (*stop)(int fd); memset(mot,0,sizeof(mot)); - if ( 0 && _System_state_Is_up(_System_state_Get()) ) { + if ( _System_state_Is_up(_System_state_Get()) ) { read_bytes = read; stop = close; fd = open(BSP_I2C_VPD_EEPROM_DEV_NAME, 0);
_______________________________________________ rtems-devel mailing list rtems-devel@rtems.org http://www.rtems.org/mailman/listinfo/rtems-devel