Re: [PATCH RFC 10/15] x86: add an arch helper function to invalidate all cache for nvdimm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/10/2022 2:09 PM, Mark Rutland wrote:
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 for the explanation.  Really does seem that
ARM could use the equivalent on wbnoinvd/wbinvd/invd.

Regards - Eliot



[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux