[PATCH 5/6] arm64: Implement pmem API support

[Dan Williams]

On Fri, Aug 4, 2017 at 11:35 AM, Robin Murphy <robin.murphy at arm.com> wrote:
> On 04/08/17 19:09, Dan Williams wrote:
>> On Fri, Aug 4, 2017 at 10:43 AM, Robin Murphy <robin.murphy at 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.
>

The mapping type of block-apertures is up to the architecture, so you
could mark them uncacheable and not worry about mmio_flush_range().
Also, arch_wb_pmem() is not a replacement for mmio_flush_range() since
we also need the cache to be invalidated. arch_wb_pmem() is allowed to
leave clean cache lines present.