This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
RE: MPC8260 cache patch
- From: Gary Thomas <gary at mlbassoc dot com>
- To: Patrick Doyle <wpd at delcomsys dot com>
- Cc: eCos patches <ecos-patches at sources dot redhat dot com>
- Date: 28 Mar 2003 07:33:35 -0700
- Subject: RE: MPC8260 cache patch
- References: <NFBBJAJICAKJPMMKDAGBEELBDNAA.wpd@delcomsys.com>
On Fri, 2003-03-28 at 07:20, Patrick Doyle wrote:
> OK, I'll try to put it together over the weekend and send out a patch
> Monday.
>
> Just as a sanity check, do you think that removing the 'INVALIDATE()' calls
> from the 'HAL_FLASH_CACHES_OFF()' macro is still worth doing? I think it
> should be safe (note the big difference between "I think" and "I have proved
> conclusively" here). Or do you think we should leave them in?
>
Yes, I also think that it's safe in this case.
> --wpd
>
>
> > -----Original Message-----
> > From: Gary Thomas [mailto:gary at mlbassoc dot com]
> > Sent: Friday, March 28, 2003 7:35 AM
> > To: Patrick Doyle
> > Cc: eCos patches
> > Subject: RE: MPC8260 cache patch
> >
> >
> > On Thu, 2003-03-27 at 11:53, Patrick Doyle wrote:
> > > 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).
> > >
> >
> > I don't see a good way around this either (I've researched what's
> > happening
> > in the LinuxPPC world as well). I'd say that the best solution, certainly
> > the easiest, is just to double the reads (pretend the cache is twice the
> > size) for the SYNC() operation. This will always work, based on how the
> > PowerPC caches function.
> >
> > Note: there are currently only two places in eCos where this is "used".
> > Both are rare - FLASH operations (which take forever anyway) and when
> > booting a Linux kernel (because one normally has to hand off the machine
> > with the caches off). Neither of these is time-critical nor repeated
> > often.
> >
> > So, Patrick, please resurrect your original patch. Can you make it
> > for all PowerPC SYNC() macros as well?
> >
> > Thanks.
> >
> > > --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
> > > > ------------------------------------------------------------
> > > >
> > > ----
> > >
> >
> > > Index: packages/io/flash/current/ChangeLog
> > > ===================================================================
> > > RCS file: /cvs/ecos/ecos/packages/io/flash/current/ChangeLog,v
> > > retrieving revision 1.26
> > > diff -u -5 -p -r1.26 ChangeLog
> > > --- packages/io/flash/current/ChangeLog 19 Mar 2003
> > 14:14:56 -0000 1.26
> > > +++ packages/io/flash/current/ChangeLog 27 Mar 2003 18:43:25 -0000
> > > @@ -1,5 +1,12 @@
> > > +2003-03-27 Patrick Doyle <wpd at delcomsys dot com>
> > > +
> > > + * include/flash.h (HAL_FLASH_CACHES_OFF): Removed overzealous
> > > + calls to 'HAL_DCACHE_INVALIDATE_ALL()' and
> > > + 'HAL_ICACHE_INVALIDATE_ALL()'.
> > > +
> > > +
> > > 2003-03-19 Thomas Koeller <thomas dot koeller at baslerweb dot com>
> > >
> > > * src/flashiodev.c: Fixed trivial error.
> > >
> > > 2003-03-03 Knud Woehler <knud dot woehler at microplex dot de>
> > > Index: packages/io/flash/current/include/flash.h
> > > ===================================================================
> > > RCS file: /cvs/ecos/ecos/packages/io/flash/current/include/flash.h,v
> > > retrieving revision 1.15
> > > diff -u -5 -p -r1.15 flash.h
> > > --- packages/io/flash/current/include/flash.h 23 May 2002
> > 23:06:14 -0000 1.15
> > > +++ packages/io/flash/current/include/flash.h 27 Mar 2003
> > 18:43:25 -0000
> > > @@ -202,13 +202,11 @@ externC cyg_bool plf_flash_query_soft_wp
> > > #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
> > >
> > > #define HAL_FLASH_CACHES_ON(_d_, _i_) \
> > > CYG_MACRO_START \
> > > if (_d_) HAL_DCACHE_ENABLE(); \
> > --
> > ------------------------------------------------------------
> > 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
------------------------------------------------------------