From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH 6/7] xen/arm: flush D-cache and I-cache when appropriate Date: Tue, 13 Nov 2012 12:15:49 +0000 Message-ID: <20121113121549.GD44675@ocelot.phlegethon.org> References: <20121024155945.GD39126@ocelot.phlegethon.org> <20121026090141.GB76080@ocelot.phlegethon.org> <20121026165549.GF76080@ocelot.phlegethon.org> <20121027104428.GB89901@ocelot.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: "xen-devel@lists.xensource.com" , Ian Campbell List-Id: xen-devel@lists.xenproject.org At 12:01 +0000 on 13 Nov (1352808094), Stefano Stabellini wrote: > > I think we should have two functions. One should look almost like that > > and be for flushing large ranges, and one much simpler for flushing > > small items. Like this (totally untested, uncompiled even): > > > > #define MIN_CACHELINE_BYTES 32 // or whatever > > > > /* In setup.c somewhere. */ > > if ( READ_CP32(CCSIDR) < MIN_CACHELINE_BYTES ) > > panic("CPU has preposterously small cache lines"); > > > > /* Function for flushing medium-sized areas. > > * if 'range' is large enough we might want to use model-specific > > * full-cache flushes. */ > > static inline void flush_xen_dcache_va_range(void *p, unsigned long size) > > { > > void *end; > > unsigned long cacheline_bytes = READ_CP32(CCSIDR); > > barrier(); /* So the compiler issues all writes to the range */ > > dsb(); /* So the CPU issues all writes to the range */ > > for ( end = p + size; p < end; p += cacheline_bytes ) > > WRITE_CP32(DCCMVAC, p); > > dsb(); /* So we know the flushes happen before continuing */ > > } > > > > /* Macro for flushing a single small item. The predicate is always > > * compile-time constant so this will compile down to 3 instructions in > > * the common case. Make sure to call it with the correct type of > > * pointer! */ > > #define flush_xen_dcache_va(p) do { \ > > typeof(p) _p = (p); \ > > if ( (sizeof *_p) > MIN_CACHELINE_BYTES ) \ > > flush_xen_dcache_va_range(_p, sizeof *_p); \ > > else \ > > asm volatile ( \ > > "dsb;" /* Finish all earlier writes */ \ > > STORE_CP32(0, DCCMVAC) \ > > "dsb;" /* Finish flush before continuing */ \ > > : : "r" (_p), "m" (*_p)); \ > > } while (0) > > > > What do you think? > > I think that is OK, but I would like to avoid reading CCSIDR every > single time we need to do a dcache flush The code above only reads it once for each large dcache flush. When I was writing it I did think of just stashing cacheline_bytes in a read_mostly somewhere, but I had the opposite concern -- wouldn't reading this constant from on-chip be quicker than going to the memory bus for it? :) I'm happy either way. Tim.