[Sound-open-firmware] cache invalidation evicting wrong lines

[Liam Girdwood]

On Fri, 2019-03-15 at 17:56 +0100, Guennadi Liakhovetski wrote:
> > Could the list be corrupt ? I cant see where cacscade_list is invalidated ?
> 
> list_for_item_smp does that:
> 
> #define list_for_item_smp(item, list) \
>         dcache_invalidate_region(list, sizeof(*(list))); \
>         for (item = (list)->next, dcache_invalidate_region(item, \
>                                                            sizeof(*(item))); \
>              item != list; \
>              item = (item)->next, dcache_invalidate_region(item, \
>                                                            sizeof(*(item))))
> 
> > >              struct irq_cascade_desc *c = container_of(list,
> > >                                              struct irq_cascade_desc,
> > > list);
> > > 
> > > // One of these kills "cascade"

Another issue could possible be the compiler re-ordering some of these
statements above ? Try removing -O2.

Btw, are you writing back prior to releasing the lock on the data ?

> > 
> > Is cascade killed on the first list iteration or a subsequent iteration. Is
> > this
> > iteration random or always consistent. Does this bug location change with
> > debug
> > code added ?
> 
> It seemed to be consistent. First 4 "level2" - "level5" cascading controllers
> are
> added, all is good then. Then 2 INTC controllers are added - for the low and
> the
> high 32 bits respectively. When the first one of those is added and when the
> list
> of already installed controllers is checked, on a specific one of them the
> .irq
> gets invalidated.

Also see (cacheasm.h), called by dcache_invalidate(), this is taking care of
cache line alignment for address and size.

/*
 *  Macro to apply a 'hit' cache instruction to a memory region,
 *  ie. to any cache entries that cache a specified portion (region) of memory.
 *  Takes care of the unaligned cases, ie. may apply to one
 *  more cache line than $asize / lineSize if $aaddr is not aligned.
 *
 *
 *  Parameters are:
 *	cainst	instruction/macro that takes an address register parameter
 *		and an offset parameter (currently always zero)
 *		and generates a cache instruction (eg. "dhi", "dhwb", "ihi", etc.)
 *	linesize_log2	log2(size of cache line in bytes)
 *	addr	register containing start address of region (clobbered)
 *	asize	register containing size of the region in bytes (clobbered)
 *	askew	unique register used as temporary
 *	awb		unique register used as temporary for erratum 497.
 *
 *  Note: A possible optimization to this macro is to apply the operation
 *  to the entire cache if the region exceeds the size of the cache
 *  by some empirically determined amount or factor.  Some experimentation
 *  is required to determine the appropriate factors, which also need
 *  to be tunable if required.
 */

	.macro	cache_hit_region	cainst, linesize_log2, addr, asize, askew, awb=a0

	//  Make \asize the number of iterations:
	extui	\askew, \addr, 0, \linesize_log2	// get unalignment amount of \addr
	add	\asize, \asize, \askew			// ... and add it to \asize
	addi	\asize, \asize, (1 << \linesize_log2) - 1	// round up!
	srli	\asize, \asize, \linesize_log2

	//  Iterate over region:
	.ifne	((\awb !=a0) & XCHAL_ERRATUM_497)		// don't use awb if set to a0
	movi \awb, 0
	.endif
	floopnez	\asize, cacheh\@
	\cainst		\addr, 0
	.ifne	((\awb !=a0) & XCHAL_ERRATUM_497)		// do memw after 8 cainst wb instructions
	addi \awb, \awb, 1
	blti \awb, 8, .Lstart_memw\@
	memw
	movi \awb, 0
.Lstart_memw\@:
	.endif
	addi		\addr, \addr, (1 << \linesize_log2)	// move to next line
	floopend	\asize, cacheh\@
	.endm

Liam