Re: [RFC PATCH 12/15] kmap: Add stray write protection for device pages

[Dave Hansen]

On 7/14/20 12:29 PM, Peter Zijlstra wrote:
> On Tue, Jul 14, 2020 at 12:06:16PM -0700, Ira Weiny wrote:
>> On Tue, Jul 14, 2020 at 10:44:51AM +0200, Peter Zijlstra wrote:
>>> So, if I followed along correctly, you're proposing to do a WRMSR per
>>> k{,un}map{_atomic}(), sounds like excellent performance all-round :-(
>> Only to pages which have this additional protection, ie not DRAM.
>>
>> User mappings of this memory is not affected (would be covered by User PKeys if
>> desired).  User mappings to persistent memory are the primary use case and the
>> performant path.
> Because performance to non-volatile memory doesn't matter? I think Dave
> has a better answer here ...

So, these WRMSRs are less evil than normal.  They're architecturally
non-serializing instructions, just like the others in the SDM WRMSR
documentation:

	Note that WRMSR to the IA32_TSC_DEADLINE MSR (MSR index 6E0H)
	and the X2APIC MSRs (MSR indices 802H to 83FH) are  not
	serializing.

This section of the SDM needs to be updated for the PKRS.  Also note
that the PKRS WRMSR is similar in its ordering properties to WRPKRU:

	WRPKRU will never execute speculatively. Memory accesses
	affected by PKRU register will not execute (even speculatively)
	until all prior executions of WRPKRU have completed execution
	and updated the PKRU register.

Which means we don't have to do silliness like LFENCE before WRMSR to
get ordering *back*.  This is another tidbit that needs to get added to
the SDM.  It should probably also get captured in the changelog.

But, either way, this *will* make accessing PMEM more expensive from the
kernel.  No escaping that.  But, we've also got customers saying they
won't deploy PMEM until we mitigate this stray write issue.  Those folks
are quite willing to pay the increased in-kernel cost for increased
protection from stray kernel writes.  Intel is also quite motivated
because we really like increasing the number of PMEM deployments. :)

Ira, can you make sure this all gets pulled into the changelogs somewhere?