On 04/08/17 19:09, Dan Williams wrote: > On Fri, Aug 4, 2017 at 10:43 AM, Robin Murphy <robin.mur...@arm.com> wrote: >> On 04/08/17 16:25, Catalin Marinas wrote: >>> Two minor comments below. >>> >>> On Tue, Jul 25, 2017 at 11:55:42AM +0100, Robin Murphy wrote: >>>> --- a/arch/arm64/Kconfig >>>> +++ b/arch/arm64/Kconfig >>>> @@ -960,6 +960,17 @@ config ARM64_UAO >>>> regular load/store instructions if the cpu does not implement the >>>> feature. >>>> >>>> +config ARM64_PMEM >>>> + bool "Enable support for persistent memory" >>>> + select ARCH_HAS_PMEM_API >>>> + help >>>> + Say Y to enable support for the persistent memory API based on the >>>> + ARMv8.2 DCPoP feature. >>>> + >>>> + The feature is detected at runtime, and the kernel will use DC CVAC >>>> + operations if DC CVAP is not supported (following the behaviour of >>>> + DC CVAP itself if the system does not define a point of >>>> persistence). >>> >>> Any reason not to have this default y? >> >> Mostly because it's untested, and not actually useful without some way >> of describing persistent memory regions to the kernel (I'm currently >> trying to make sense of what exactly ARCH_HAS_MMIO_FLUSH is supposed to >> mean in order to enable ACPI NFIT support). > > This is related to block-aperture support described by the NFIT where > a sliding-memory-mapped window can be programmed to access different > ranges of the NVDIMM. Before the window is programmed to a new > DIMM-address we need to flush any dirty data through the current > window setting to media. See the call to mmio_flush_range() in > acpi_nfit_blk_single_io(). I think it's ok to omit ARCH_HAS_MMIO_FLUSH > support, and add a configuration option to compile out the > block-aperture support.
Oh, I have every intention of implementing it one way or another if necessary - it's not difficult, it's just been a question of working through the NFIT code to figure out the subtleties of translation to arm64 ;) If mmio_flush_range() is for true MMIO (i.e. __iomem) mappings, then arm64 should only need a barrier, rather than actual cache operations. If on the other hand it's misleadingly named and only actually used on MEMREMAP_WB mappings (as I'm staring to think it might be), then I can't help thinking it could simply go away in favour of arch_wb_pmem(), since that now seems to have those same semantics and intent, plus a much more appropriate name. Robin. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm