[PATCH] arm64: kill flush_cache_all()

[mark.rutland@arm.com (Mark Rutland)]

On Mon, Apr 20, 2015 at 06:23:00PM +0100, Ard Biesheuvel wrote:
> On 20 April 2015 at 11:24, Mark Rutland <mark.rutland@arm.com> wrote:
> > The documented semantics of flush_cache_all are not possible to provide
> > for arm64 (short of flushing the entire physical address space by VA),
> > and there are currently no users; KVM uses VA maintenance exclusively,
> > cpu_reset is never called, and the only two users outside of arch code
> > cannot be built for arm64.
> >
> > While cpu_soft_reset and related functions (which call flush_cache_all)
> > were thought to be useful for kexec, their current implementations only
> > serve to mask bugs. For correctness kexec will need to perform
> > maintenance by VA anyway to account for system caches, line migration,
> > and other subtleties of the cache architecture. As the extent of this
> > cache maintenance will be kexec-specific, it should probably live in the
> > kexec code.
> >
> > This patch removes flush_cache_all, and related unused components,
> > preventing further abuse.
> >
> 
> While I agree fully with the general purpose of this patch, i.e., to
> prevent set/way operations to be abused for managing coherency,
> perhaps it would make sense to retain/repurpose some of this code as
> the 'supported' way of putting the CPU in the mode that is mandated by
> the boot protocol, so that it can be shared between the EFI stub and
> kexec.
> 
> I think the former is not entirely safe at the moment, since the only
> region we clean/invalidate [by VA] is the memory region containing the
> copied kernel image, and the remainder of the efi-entry.S code itself.
> I may be wrong, but I think that means that any cached software state
> owned by the runtime services [including virtual remappings] could
> potentially get lost when we disable the dcache without cleaning it by
> set/way first.

Regarding the boot protocol, note that this was relaxed a while back
w.r.t. cache requirements, only mandating that certain regions of memory
are clean to the PoC.

Clearing SCTLR.C prevents accesses from being given cacheable
attributes, but doesn't disable any caches as such. The CPU caches
remain active and coherent (unless some IMPLEMENTATION DEFINED mechanism
is used to exit coherency), so the data will remain there unless
naturally evicted, taken by another cache, or explicitly
cleaned/invalidated.

With SCTLR.C clear, memory accesses should not result in the allocation
of new lines into the caches (though if data for a given address is
present, an access may hit said data). Clearing SCTLR.M is necessary to
prevent allocation by the page table walkers, unless these are not using
cacheable attributes to begin with.

Set/Way maintenance races with the usual operation of the caches while
cacheable accesses may be made into them, so do not make sense to use
while SCTLR.C or SCTLR.M are set. They can only drain the CPU-local
caches when nothing may allocate into them, and do not affect system
caches, so portable software must use maintenance by VA for correctness.

Does that address your concerns? Or pose new questions? ;)

Thanks,
Mark.