From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aneesh V Date: Fri, 06 Jul 2012 20:00:06 -0700 Subject: [U-Boot] [PATCH 2/9] CACHE: Add cache_aligned() macro In-Reply-To: <201206260130.38239.marex@denx.de> References: <1340583477-14018-1-git-send-email-marex@denx.de> <1340583477-14018-3-git-send-email-marex@denx.de> <4FE8D42D.1000400@freescale.com> <201206260130.38239.marex@denx.de> Message-ID: <4FF7A636.10106@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Marek, On 06/25/2012 04:30 PM, Marek Vasut wrote: > Dear Scott Wood, > >> On 06/24/2012 07:17 PM, Marek Vasut wrote: >>> This macro returns 1 if the argument (address) is aligned, returns >>> zero otherwise. This will be used to test user-supplied address to >>> various commands to prevent user from loading data to/from unaligned >>> address when using caches. >>> >>> This is made as a macro, because macros are expanded where they are >>> used. Therefore it can be easily instrumented to report position of >>> the fault. >>> >>> Signed-off-by: Marek Vasut >>> Cc: Wolfgang Denk >>> --- >>> >>> include/common.h | 18 ++++++++++++++++++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/include/common.h b/include/common.h >>> index 322569e..17c64b0 100644 >>> --- a/include/common.h >>> +++ b/include/common.h >>> @@ -730,6 +730,24 @@ void invalidate_dcache_range(unsigned long start, >>> unsigned long stop); >>> >>> void invalidate_dcache_all(void); >>> void invalidate_icache_all(void); >>> >>> +/* Test if address is cache-aligned. Returns 0 if it is, 1 otherwise. */ >>> +#define cacheline_aligned(addr) > \ >>> + ({ \ >>> + int __ret; \ >>> + if (!dcache_status()) { \ >>> + __ret = 1; \ >>> + } else if ((addr)& (ARCH_DMA_MINALIGN - 1)) { \ >>> + puts("Align load address to " \ >>> + __stringify(ARCH_DMA_MINALIGN) \ >>> + " bytes when using caches!\n"); \ >>> + __ret = 0; \ >>> + } else { \ >>> + __ret = 1; \ >>> + } \ >>> + __ret; \ >>> + }) >> >> What if it's a store rather than a load? > > Goot point #1 > >> If this is only supposed to be >> used for loads (because on a store you can flush the cache rather than >> invalidate), it's not labelled that way, the changelog says "to/from >> unaligned address", and the caller might be common code that doesn't >> know which direction the transfer is in. > > And we're back at the flush/invalidate thing. This is what I'd really love to > understand once and for all: > > 1) We must prevent user from loading to address 0x.......1 (unaligned), I'll get > back to this later on. > > 2) If the user had address that starts at aligned location but ends at > unaligned: > > Terminology: > va -- unrelated variable > [****] -- aligned range > > 2a) LOAD from external source to local address: > The cache must be invalidated over that area. I see the following issue: > > [---------buffer---------][va] > [********][********][********] > So consider the scenario where > i) va is written in u-boot > ii) data arrive into the buffer via DMA at the same time > iii) cache is invalidated over whole buffer > > This will cause variable "va" to be lost forever. The "va" variable might as > well be user data, therefore in order to protect those, we should first flush > the user-supplied area. Flushing the last cache line may corrupt the data you just DMAed in. The current implementation after a lot of discussions [1] with Albert is this: Invalidate only the aligned part of the buffer. So, in the above case the first two [********] will get invalidated not the last one. The last un-aligned cache line will result in a warning. [1] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/105113 > > 2b) STORE from local address to external source: > Basically the inverted problem as in 2a), but if the driver is written properly, > shouldn't be any issue. No. Store doesn't have any problem because flush doesn't harm anything. I have laid down the invalidate/flush requirements for DMA buffers in: doc/README.arm-caches best regards, Aneesh