On Thu, 2019-08-15 at 09:29 +0200, christophe leroy wrote: > > Le 15/08/2019 à 06:10, Alastair D'Silva a écrit : > > From: Alastair D'Silva <alast...@d-silva.org> > > > > Similar to commit 22e9c88d486a > > ("powerpc/64: reuse PPC32 static inline flush_dcache_range()") > > this patch converts flush_icache_range() to C, and reimplements the > > following functions as wrappers around it: > > __flush_dcache_icache > > __flush_dcache_icache_phys > > Not sure you can do that for __flush_dcache_icache_phys(), see > detailed > comments below > > > This was done as we discovered a long-standing bug where the length > > of the > > range was truncated due to using a 32 bit shift instead of a 64 bit > > one. > > > > By converting these functions to C, it becomes easier to maintain. > > > > Signed-off-by: Alastair D'Silva <alast...@d-silva.org> > > --- > > arch/powerpc/include/asm/cache.h | 26 +++--- > > arch/powerpc/include/asm/cacheflush.h | 32 ++++--- > > arch/powerpc/kernel/misc_32.S | 117 ------------------- > > ------- > > arch/powerpc/kernel/misc_64.S | 97 --------------------- > > arch/powerpc/mm/mem.c | 71 +++++++++++++++- > > 5 files changed, 102 insertions(+), 241 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/cache.h > > b/arch/powerpc/include/asm/cache.h > > index f852d5cd746c..728f154204db 100644 > > --- a/arch/powerpc/include/asm/cache.h > > +++ b/arch/powerpc/include/asm/cache.h > > @@ -98,20 +98,7 @@ static inline u32 l1_icache_bytes(void) > > #endif > > #endif /* ! __ASSEMBLY__ */ > > > > -#if defined(__ASSEMBLY__) > > -/* > > - * For a snooping icache, we still need a dummy icbi to purge all > > the > > - * prefetched instructions from the ifetch buffers. We also need a > > sync > > - * before the icbi to order the the actual stores to memory that > > might > > - * have modified instructions with the icbi. > > - */ > > -#define PURGE_PREFETCHED_INS \ > > - sync; \ > > - icbi 0,r3; \ > > - sync; \ > > - isync > > Is this still used anywhere now ? No, this patch removes all users of it.
> > - > > -#else > > +#if !defined(__ASSEMBLY__) > > #define __read_mostly > > __attribute__((__section__(".data..read_mostly"))) > > > > #ifdef CONFIG_PPC_BOOK3S_32 > > @@ -145,6 +132,17 @@ static inline void dcbst(void *addr) > > { > > __asm__ __volatile__ ("dcbst %y0" : : "Z"(*(u8 *)addr) : > > "memory"); > > } > > + > > +static inline void icbi(void *addr) > > +{ > > + __asm__ __volatile__ ("icbi 0, %0" : : "r"(addr) : "memory"); > > +} > > + > > +static inline void iccci(void) > > +{ > > + __asm__ __volatile__ ("iccci 0, r0"); > > I think you need the "memory" clobber too here. > Thanks, I'll add that. > > +} > > + > > #endif /* !__ASSEMBLY__ */ > > #endif /* __KERNEL__ */ > > #endif /* _ASM_POWERPC_CACHE_H */ > > diff --git a/arch/powerpc/include/asm/cacheflush.h > > b/arch/powerpc/include/asm/cacheflush.h > > index ed57843ef452..4c3377aff8ed 100644 > > --- a/arch/powerpc/include/asm/cacheflush.h > > +++ b/arch/powerpc/include/asm/cacheflush.h > > @@ -42,24 +42,18 @@ extern void flush_dcache_page(struct page > > *page); > > #define flush_dcache_mmap_lock(mapping) do { } while > > (0) > > #define flush_dcache_mmap_unlock(mapping) do { } while (0) > > > > -extern void flush_icache_range(unsigned long, unsigned long); > > +void flush_icache_range(unsigned long start, unsigned long stop); > > extern void flush_icache_user_range(struct vm_area_struct *vma, > > struct page *page, unsigned long > > addr, > > int len); > > -extern void __flush_dcache_icache(void *page_va); > > extern void flush_dcache_icache_page(struct page *page); > > -#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE) > > -extern void __flush_dcache_icache_phys(unsigned long physaddr); > > -#else > > -static inline void __flush_dcache_icache_phys(unsigned long > > physaddr) > > -{ > > - BUG(); > > -} > > -#endif > > > > -/* > > - * Write any modified data cache blocks out to memory and > > invalidate them. > > +/** > > + * flush_dcache_range(): Write any modified data cache blocks out > > to memory and invalidate them. > > * Does not invalidate the corresponding instruction cache > > blocks. > > + * > > + * @start: the start address > > + * @stop: the stop address (exclusive) > > */ > > static inline void flush_dcache_range(unsigned long start, > > unsigned long stop) > > { > > @@ -82,6 +76,20 @@ static inline void flush_dcache_range(unsigned > > long start, unsigned long stop) > > isync(); > > } > > > > +/** > > + * __flush_dcache_icache(): Flush a particular page from the data > > cache to RAM. > > + * Note: this is necessary because the instruction cache does > > *not* > > + * snoop from the data cache. > > + * > > + * @page: the address of the page to flush > > + */ > > +static inline void __flush_dcache_icache(void *page) > > +{ > > + unsigned long page_addr = (unsigned long)page; > > The function is small enough to call this addr instead of page_addr > without ambiguity. Ok > > > + > > + flush_icache_range(page_addr, page_addr + PAGE_SIZE); > > Finally, I think that's not so simple. For the 44x, if > MMU_FTR_TYPE_44x > is set, that's just a clean_dcache_range(). > If that feature is not set, it looks like a standard > flush_icache_range() but without the special CONFIG_4xx handling > with > iccci we have in flush_icache_range() > Hmm, I'll have to hack at this a bit more. <snip> -- Alastair D'Silva Open Source Developer Linux Technology Centre, IBM Australia mob: 0423 762 819