(cc Marc) On Thu, 4 Feb 2021 at 11:48, Russell King - ARM Linux admin <[email protected]> wrote: > > On Thu, Feb 04, 2021 at 11:27:16AM +0100, Ard Biesheuvel wrote: > > Hi Russell, > > > > If Guillaume is willing to do the experiment, and it fixes the issue, > > it proves that rk3288 is relying on the flush before the MMU is > > disabled, and so in that case, the fix is trivial, and we can just > > apply it. > > > > If the experiment fails (which would mean rk3288 does not tolerate the > > cache maintenance being performed after cache off), it is going to be > > hairy, and so it will definitely take more time. > > > > So in the latter case (or if Guillaume does not get back to us), I > > think reverting my queued fix is the only sane option. But in that > > case, may I suggest that we queue the revert of the original by-VA > > change for v5.12 so it gets lots of coverage in -next, and allows us > > an opportunity to come up with a proper fix in the same timeframe, and > > backport the revert and the subsequent fix as a pair? Otherwise, we'll > > end up in the situation where v5.10.x until today has by-va, v5.10.x-y > > has set/way, and v5.10y+ has by-va again. (I don't think we care about > > anything before that, given that v5.4 predates any of this) > > I'm suggesting dropping your fix (9052/1) and reverting > "ARM: decompressor: switch to by-VA cache maintenance for v7 cores" > which gets us to a point where _both_ regressions are fixed. >
I understand, but we don't know whether doing so might regress other platforms that were added in the mean time. > I'm of the opinion that the by-VA patch was incorrect when it was > merged (it caused a regression), and it's only a performance > improvement. It is a correctness improvement, not a performance improvement. Without that change, the 32-bit ARM kernel cannot boot bare metal on platforms with a system cache such as 8040 or SynQuacer, and can only boot under KVM on such systems because of the special handling of set/way instructions by the host. The performance issue related to set/way ops under KVM was already fixed by describing data and unified caches as 1 set and 1 way when running in 32-bit mode. > Our attempts so far to fix it are just causing other > regressions. So, I think it is reasonable to revert both back to a > known good point which has worked over a decade. If doing so causes > regressions (which I think is unlikely), then that would be unfortunate > but alas is a price that's worth paying to get back to a known good > point - since then we're not stacking regression fixes on top of other > regression fixes. > This is exactly why I am proposing to queue the revert of the original patch for v5.12, and only backport it to v5.10 and v5.11 once we are sure it does not break anything else.
