Port over the Exynos cacheline size fix from Dolphin. #8965
Conversation
…m of the mono project for the discovery and original fix. See dolphin-emu/dolphin#4204 and mono/mono#3549
|
|
||
| // use the global minimum cache line size | ||
| icache_line_size = isize = icache_line_size < isize ? icache_line_size : isize; | ||
| dcache_line_size = dsize = dcache_line_size < dsize ? dcache_line_size : dsize; |
There was a problem hiding this comment.
Hmm, if we're not using the line size we just read, then this can mean a race condition, right?
For example, let's say FlushIcacheSection() is called on BIG cpu, possibly several times. When it swaps to the LITTLE cpu:
- If it's before we check the line size, it's fine, we'll get the right one.
- If it's after we invalidate, then hopefully it's handled correctly.
- If it's while we're invalidating, we're screwed if it doesn't handle the swap correctly. We still have the BIG size (having never been exposed to the LITTLE core.)
If it's necessary to take the lowest, then we must need to re-read the line size for every iteration of the loop. It seems to me we shouldn't cache at all?
-[Unknown]
There was a problem hiding this comment.
yes, this is correct (it's unlikely though). Even if you would read it on every loop iteration, there would be still a race, because the loop iteration itself isn't atomic.
The right way would be to iterate over all available cores on startup and determine the minimum cache line size. I couldn't figure out a reliable way to do that on Android. Or, since you already have infrastructure for device specific quirks, you could hard code it...
There was a problem hiding this comment.
But... since we save the minimum, and a core jump will wipe the icache anyway, case 3 can't possibly be an issue?
There was a problem hiding this comment.
hah, good point! maybe we don't need to try to figure out the minimum cache line size after all then.
There was a problem hiding this comment.
And, reverted it. Was convinced by some comments in the HN thread - in case the cores are used simultaneously it might still be an issue.
|
Yeah, I guess I should've looked at gcc's code - I suspected it was invalidating wrong but thought both CPUs had separate icaches that weren't being invalidated on swap, or something. I think given this, the existing fix is not good enough. If a jit block is larger than a cache line, the padding won't solve it.. it's just an alignment thing. It only happens to fix most of the cases by luck. I definitely agree we should merge this before v1.3.0. It looks like it's been confirmed in Dolphin that this fixes things there. -[Unknown] |
Thanks to lewurm of the mono project for the discovery and original fix.
Should finally and correctly fix, rather than work around, the crashing issues seen on non-US Galaxy S7 devices (fixes #8908)
See dolphin-emu/dolphin#4204 and mono/mono#3549
Replaces #8769 (which was a good effort!)