On Wed, Aug 10, 2022 at 10:31:12AM -0400, Eliot Moss wrote: > On 8/10/2022 10:15 AM, Mark Rutland wrote: > > On Tue, Aug 09, 2022 at 02:47:06PM -0700, Dave Jiang wrote: > > > > > > On 8/3/2022 10:37 AM, Jonathan Cameron wrote: > > > > On Tue, 19 Jul 2022 12:07:03 -0700 > > > > Dave Jiang <dave.jiang@xxxxxxxxx> wrote: > > > > > > > > > On 7/17/2022 10:30 PM, Davidlohr Bueso wrote: > > > > > > On Fri, 15 Jul 2022, Dave Jiang wrote: > > > > > > > The original implementation to flush all cache after unlocking the > > > > > > > nvdimm > > > > > > > resides in drivers/acpi/nfit/intel.c. This is a temporary stop gap until > > > > > > > nvdimm with security operations arrives on other archs. With support CXL > > > > > > > pmem supporting security operations, specifically "unlock" dimm, the > > > > > > > need > > > > > > > for an arch supported helper function to invalidate all CPU cache for > > > > > > > nvdimm has arrived. Remove original implementation from acpi/nfit and > > > > > > > add > > > > > > > cross arch support for this operation. > > > > > > > > > > > > > > Add CONFIG_ARCH_HAS_NVDIMM_INVAL_CACHE Kconfig and allow x86_64 to > > > > > > > opt in > > > > > > > and provide the support via wbinvd_on_all_cpus() call. > > > > > > So the 8.2.9.5.5 bits will also need wbinvd - and I guess arm64 will need > > > > > > its own semantics (iirc there was a flush all call in the past). Cc'ing > > > > > > Jonathan as well. > > > > > > > > > > > > Anyway, I think this call should not be defined in any place other > > > > > > than core > > > > > > kernel headers, and not in pat/nvdimm. I was trying to make it fit in > > > > > > smp.h, > > > > > > for example, but conviniently we might be able to hijack > > > > > > flush_cache_all() > > > > > > for our purposes as of course neither x86-64 arm64 uses it :) > > > > > > > > > > > > And I see this as safe (wrt not adding a big hammer on unaware > > > > > > drivers) as > > > > > > the 32bit archs that define the call are mostly contained thin their > > > > > > arch/, > > > > > > and the few in drivers/ are still specific to those archs. > > > > > > > > > > > > Maybe something like the below. > > > > > Ok. I'll replace my version with yours. > > > > Careful with flush_cache_all(). The stub version in > > > > include/asm-generic/cacheflush.h has a comment above it that would > > > > need updating at very least (I think). > > > > Note there 'was' a flush_cache_all() for ARM64, but: > > > > https://patchwork.kernel.org/project/linux-arm-kernel/patch/1429521875-16893-1-git-send-email-mark.rutland@xxxxxxx/ > > > > > > > > > flush_and_invalidate_cache_all() instead given it calls wbinvd on x86? I > > > think other archs, at least ARM, those are separate instructions aren't > > > they? > > > > On arm and arm64 there is no way to perform maintenance on *all* caches; it has > > to be done in cacheline increments by address. It's not realistic to do that > > for the entire address space, so we need to know the relevant address ranges > > (as per the commit referenced above). > > > > So we probably need to think a bit harder about the geenric interface, since > > "all" isn't possible to implement. :/ > > Can you not do flushing by set and way on each cache, > probably working outwards from L1? Unfortunately, for a number of reasons, that doeesn't work. For better or worse, the *only* way which is guaranteed to work is to do this by address. If you look at the latest ARM ARM (ARM DDI 0487H.a): https://developer.arm.com/documentation/ddi0487/ha/ ... on page D4-4754, in the block "Example code for cache maintenance instructions", there's note with a treatise on this. The gist is that: * Set/Way ops are only guaranteed to affect the caches local to the CPU issuing them, and are not guaranteed to affect caches owned by other CPUs. * Set/Way ops are not guaranteed to affect system-level caches, which are fairly popular these days (whereas VA ops are required to affect those). * Set/Way ops race with the natural behaviour of caches (so e.g. a line could bounce between layers of cache, or between caches in the system, and avoid being operated upon). So unless you're on a single CPU system, with translation disabled, and you *know* that there are no system-level caches, you can't rely upon Set/Way ops to do anything useful. They're really there for firmware to use for IMPLEMENTATION DEFINED power-up and power-down sequences, and aren'y useful to portable code. Thanks, Mark.
