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

From: Dan Williams <dan.j.williams@intel.com>
To: Davidlohr Bueso <dave@stgolabs.net>,
	Dan Williams <dan.j.williams@intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	<linux-cxl@vger.kernel.org>, <nvdimm@lists.linux.dev>,
	<bwidawsk@kernel.org>, <ira.weiny@intel.com>,
	<vishal.l.verma@intel.com>, <alison.schofield@intel.com>,
	<a.manzanares@samsung.com>, <linux-arch@vger.kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH RFC 10/15] x86: add an arch helper function to invalidate all cache for nvdimm
Date: Wed, 10 Aug 2022 14:30:51 -0700	[thread overview]
Message-ID: <62f4238b5ce8a_3ce6829447@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <20220810211337.ha27cl24splm4wjh@offworld>

Davidlohr Bueso wrote:
> On Wed, 10 Aug 2022, Dan Williams wrote:
> 
> >I expect the interface would not be in the "flush_cache_" namespace
> >since those functions are explicitly for virtually tagged caches that
> >need maintenance on TLB operations that change the VA to PA association.
> >In this case the cache needs maintenance because the data at the PA
> >changes. That also means that putting it in the "nvdimm_" namespace is
> >also wrong because there are provisions in the CXL spec where volatile
> >memory ranges can also change contents at a given PA, for example caches
> >might need to be invalidated if software resets the device, but not the
> >platform.
> >
> >Something like:
> >
> >    region_cache_flush(resource_size_t base, resource_size_t n, bool nowait)
> >
> >...where internally that function can decide if it can rely on an
> >instruction like wbinvd, use set / way based flushing (if set / way
> >maintenance can be made to work which sounds like no for arm64), or map
> >into VA space and loop. If it needs to fall back to that VA-based loop
> >it might be the case that the caller would want to just fail the
> >security op rather than suffer the loop latency.
> 
> Yep, I was actually prototyping something similar, but want to still
> reuse cacheflush.h machinery and just introduce cache_flush_region()
> or whatever name, which returns any error. So all the logic would
> just be per-arch, where x86 will do the wbinv and return 0, and arm64
> can just do -EINVAL until VA-based is no longer the only way.

cache_flush_region() works for me, but I wonder if there should be a
cache_flush_region_capable() call to shut off dependent code early
rather than discovering it at runtime? For example, even archs like x86,
that have wbinvd, have scenarios where wbinvd is prohibited, or painful.
TDX, and virtualization in general, comes to mind.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel