RE: MPC8260 cache patch

["Patrick Doyle" <wpd at delcomsys dot com>]

For those of you who are still interested (is anybody still interested?) I
tried removing the invalidate lines for 'HAL_FLASH_CACHES_OFF()'.  As
expected, this made the problem go away, so I have (hopefully) attached a
patch to that effect.

Here are my thoughts:
'HAL_FLASH_CACHES_OFF()' syncs the data cache and then disables it.  This
makes the cache be coherent with main memory, but does not necessarily make
main memory be coherent with the cache.  Code that calls
'HAL_FLASH_CACHES_OFF()' should always be limited to code that is going to
manipulate the flash _ONLY_ and not main memory.  (There is a potential
problem here -- the stack is in main memory, but as long as the stack is not
in the portion of main memory that is referenced by the call to
'HAL_DCACHE_SYNC()', we are ok).  So this should be safe.

There is still the underlying problem that, on the 8260, 'HAL_DCACHE_SYNC()'
does not guarantee that main memory is coherent with the cache.  I have
thought about changing the region that 'HAL_DCACHE_SYNC()' loads to be in
flash rather than SDRAM (and double checking that the flash is marked as
cacheable), but I would want to check the read timings on that vs. the burst
read timings of SDRAM to see how loading the two regions compared.  (If it
takes more than twice as long, then we might as well scan double the number
of cache lines as in my original proposal -- if it takes less time, then
we've got a winner).  I have also thought about using one of the chip select
state machines to make an alias of SDRAM that is only used for syncing the D
cache (assuming I can get away with that).  All in all, the simplest
solution is to double the number of cache lines read -- how often does the
cache need to by sync'd in a real time application?  (RedBoot doesn't count
here).

--wpd


> -----Original Message-----
> From: Gary Thomas [mailto:gary at mlbassoc dot com]
> Sent: Tuesday, March 25, 2003 9:50 AM
> To: Patrick Doyle
> Cc: eCos patches
> Subject: RE: MPC8260 cache patch
>
>
> On Tue, 2003-03-25 at 07:35, Patrick Doyle wrote:
> > In
> >
> "packages/io/flash/current/include/flash.hpackages/io/flash/curren
> t/include/
> > flash.h", there is the following definition:
> >
> > #define HAL_FLASH_CACHES_OFF(_d_, _i_)          \
> >     CYG_MACRO_START                             \
> >     _i_ = 0; /* avoids warning */               \
> >     HAL_DCACHE_IS_ENABLED(_d_);                 \
> >     HAL_DCACHE_SYNC();                          \
> >     HAL_DCACHE_INVALIDATE_ALL();                \
> >     HAL_DCACHE_DISABLE();                       \
> >     HAL_ICACHE_INVALIDATE_ALL();                \
> >     CYG_MACRO_END
> >
> > This is called in prior to the call to 'flash_dev_query()' (and
> > 'HAL_FLASH_CACHES_ON()' is called afterwards).  When it is
> called during the
> > the boot process of RedBoot, RedBoot has written some values to
> the virtual
> > vector table that do not get committed to memory by the
> (current version of)
> > 'HAL_DCACHE_SYNC()'.  Then when 'HAL_DCACHE_INVALIDATE_ALL()'
> is invoked,
> > those changes are lost.
> >
>
> I don't think that for the purposes of the FLASH code that the
> invalidates need to be there.  Notice that they are not present in
> the older version of this macro.
>
> Can you try this without the invalidate lines?
>
> > You won't get any argument from me about the desire to fully
> understand a
> > problem prior to fixing it.  There is a huge difference between fixing a
> > problem and making it go away.  In my experience, the latter practice is
> > encountered significantly more often than the former.
> >
> > OTOH,
> >
> > As I was thinking about this last night, I started to wonder,
> why does it
> > matter if the sync time is doubled?  The effeciency expert in
> my cringes to
> > hear me write this (huh?) but, seriously, how often does the
> cache need to
> > be sync'd in eCos.  We don't perform MMU-style context switches as Linux
> > does.  The caches on the '8260 support snooping (in theory -- are there
> > erratta about this not working?) so in an MP system, the
> hardware will take
> > care of coherency issues.  As I said, when I first read your
> reply saying
> > that you didn't want do double the time it takes to sync the
> cache, my first
> > reaction was one of total agreement.  But, as I thought about it more, I
> > wonder, is it really an issue for eCos?
> >
>
> We do try to keep things as slim and fast as possible; after all that's
> one of eCos' selling points!  A little here, a little there and soon
> we'll lose control.  It may be that the only safe way to do this flush
> is by doubling the lines, but I want to fully investigate first.
>
> > --wpd
> >
> >
> > > -----Original Message-----
> > > From: Gary Thomas [mailto:gary at mlbassoc dot com]
> > > Sent: Monday, March 24, 2003 6:42 PM
> > > To: Patrick Doyle
> > > Cc: eCos patches
> > > Subject: RE: MPC8260 cache patch
> > >
> > >
> > > On Mon, 2003-03-24 at 15:12, Patrick Doyle wrote:
> > > > The particular problem I was chasing down was related to the
> > > fact that the
> > > > virtual vector table is in the first 16K.  (Perhaps it could be
> > > moved for
> > > > the '8620 platform).  RedBoot initializes pointers in there
> > > that do not get
> > > > committed to main memory before the flash initialization
> > > routine invalidates
> > > > the cache.
> > > >
> > >
> > > I don't understand.  The FLASH routines don't invalidate the
> cache, only
> > > flush them.  Then they turn it off while running the code and then
> > > restore the cache [turn it back on].  There should be no need
> to access
> > > any data within that low 16K while the cache is off.  What data that's
> > > in the cache and not being flushed is being missed?
> > >
> > > This all works fine on all other platforms, including some 603 based
> > > ones.
> > >
> > > > The only other solutions I can think of (now that you
> mention the major
> > > > problem of doubling the flush time) are to not place any
> > > writable data in
> > > > the first 16K or to choose some other 16K region of cacheable
> > > memory that
> > > > could be used to flush the cache.
> > > >
> > >
> > > That's how it works on some other platforms.  The StrongARM has a
> > > special memory region which is used for only this (it's a sink hole).
> > >
> > > Note: I'm not saying that we don't need to fix this.  I just want to
> > > fully understand it first.
> > >
> > > --
> > > ------------------------------------------------------------
> > > Gary Thomas                 |
> > > MLB Associates              |  Consulting for the
> > > +1 (970) 229-1963           |    Embedded world
> > > http://www.mlbassoc.com/    |
> > > email: <gary at mlbassoc dot com>  |
> > > gpg: http://www.chez-thomas.org/gary/gpg_key.asc
> > > ------------------------------------------------------------
> > >
> --
> ------------------------------------------------------------
> Gary Thomas                 |
> MLB Associates              |  Consulting for the
> +1 (970) 229-1963           |    Embedded world
> http://www.mlbassoc.com/    |
> email: <gary at mlbassoc dot com>  |
> gpg: http://www.chez-thomas.org/gary/gpg_key.asc
> ------------------------------------------------------------
>

Attachment: flash2.txt
Description: Text document