linux-nvdimm.lists.01.org archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: tglx@linutronix.de, mingo@redhat.com
Cc: x86@kernel.org, stable@vger.kernel.org,
	Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Tony Luck <tony.luck@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Erwin Tsaur <erwin.tsaur@intel.com>,
	linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 2/2] x86/copy_safe: Introduce copy_safe_fast()
Date: Thu, 30 Apr 2020 01:25:09 -0700	[thread overview]
Message-ID: <158823510897.2094061.8955960257425244970.stgit@dwillia2-desk3.amr.corp.intel.com> (raw)
In-Reply-To: <158823509800.2094061.9683997333958344535.stgit@dwillia2-desk3.amr.corp.intel.com>

The existing copy_safe() implementation satisfies two primary concerns.
It provides a copy routine that avoids known unrecoverable scenarios
(poison consumption via fast-string instructions / accesses across
cacheline boundaries), and it provides a fallback to fast plain memcpy
if the platform does not indicate recovery capability. The fallback is
broken for two reasons. It fails the expected copy_safe() semantics that
allow it to be used in scenarios where reads / writes trigger memory
protection faults and it fails to enable machine check recovery on
systems that do not need the careful semantics of copy_safe_slow().

With copy_safe_fast() in place copy_safe() never falls back to plain
memcpy(), and it is fast regardless in every other scenario outside of
the copy_safe_slow() blacklist scenario.

Cc: x86@kernel.org
Cc: <stable@vger.kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Erwin Tsaur <erwin.tsaur@intel.com>
Tested-by: Erwin Tsaur <erwin.tsaur@intel.com>
Fixes: 92b0729c34ca ("x86/mm, x86/mce: Add memcpy_mcsafe()")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/include/asm/copy_safe.h |    2 ++
 arch/x86/lib/copy_safe.c         |    5 ++---
 arch/x86/lib/copy_safe_64.S      |   40 ++++++++++++++++++++++++++++++++++++++
 tools/objtool/check.c            |    1 +
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/copy_safe.h b/arch/x86/include/asm/copy_safe.h
index 1c130364dd61..306248ca819f 100644
--- a/arch/x86/include/asm/copy_safe.h
+++ b/arch/x86/include/asm/copy_safe.h
@@ -12,5 +12,7 @@ static inline void enable_copy_safe_slow(void)
 
 __must_check unsigned long
 copy_safe_slow(void *dst, const void *src, size_t cnt);
+__must_check unsigned long
+copy_safe_fast(void *dst, const void *src, size_t cnt);
 
 #endif /* _COPY_SAFE_H_ */
diff --git a/arch/x86/lib/copy_safe.c b/arch/x86/lib/copy_safe.c
index 9dd3a831ff94..b8472e6a44d3 100644
--- a/arch/x86/lib/copy_safe.c
+++ b/arch/x86/lib/copy_safe.c
@@ -25,7 +25,7 @@ void enable_copy_safe_slow(void)
  *
  * We only call into the slow version on systems that have trouble
  * actually do machine check recovery. Everyone else can just
- * use memcpy().
+ * use copy_safe_fast().
  *
  * Return 0 for success, or number of bytes not copied if there was an
  * exception.
@@ -34,8 +34,7 @@ __must_check unsigned long copy_safe(void *dst, const void *src, size_t cnt)
 {
 	if (static_branch_unlikely(&copy_safe_slow_key))
 		return copy_safe_slow(dst, src, cnt);
-	memcpy(dst, src, cnt);
-	return 0;
+	return copy_safe_fast(dst, src, cnt);
 }
 EXPORT_SYMBOL_GPL(copy_safe);
 
diff --git a/arch/x86/lib/copy_safe_64.S b/arch/x86/lib/copy_safe_64.S
index 46dfdd832bde..551c781ae9fd 100644
--- a/arch/x86/lib/copy_safe_64.S
+++ b/arch/x86/lib/copy_safe_64.S
@@ -2,7 +2,9 @@
 /* Copyright(c) 2016-2020 Intel Corporation. All rights reserved. */
 
 #include <linux/linkage.h>
+#include <asm/alternative-asm.h>
 #include <asm/copy_safe_test.h>
+#include <asm/cpufeatures.h>
 #include <asm/export.h>
 #include <asm/asm.h>
 
@@ -120,4 +122,42 @@ EXPORT_SYMBOL_GPL(copy_safe_slow)
 	_ASM_EXTABLE(.L_write_leading_bytes, .E_leading_bytes)
 	_ASM_EXTABLE(.L_write_words, .E_write_words)
 	_ASM_EXTABLE(.L_write_trailing_bytes, .E_trailing_bytes)
+
+/*
+ * copy_safe_fast - memory copy with exception handling
+ *
+ * Fast string copy + fault / exception handling. If the CPU does
+ * support machine check exception recovery, but does not support
+ * recovering from fast-string exceptions then this CPU needs to be
+ * added to the copy_safe_slow_key set of quirks. Otherwise, absent any
+ * machine check recovery support this version should be no slower than
+ * standard memcpy.
+ */
+SYM_FUNC_START(copy_safe_fast)
+	ALTERNATIVE "jmp copy_safe_slow", "", X86_FEATURE_ERMS
+	movq %rdi, %rax
+	movq %rdx, %rcx
+.L_copy:
+	rep movsb
+	/* Copy successful. Return zero */
+	xorl %eax, %eax
+	ret
+SYM_FUNC_END(copy_safe_fast)
+EXPORT_SYMBOL_GPL(copy_safe_fast)
+
+	.section .fixup, "ax"
+.E_copy:
+	/*
+	 * On fault %rcx is updated such that the copy instruction could
+	 * optionally be restarted at the fault position, i.e. it
+	 * contains 'bytes remaining'. A non-zero return indicates error
+	 * to copy_safe() users, or indicate short transfers to
+	 * user-copy routines.
+	 */
+	movq %rcx, %rax
+	ret
+
+	.previous
+
+	_ASM_EXTABLE_FAULT(.L_copy, .E_copy)
 #endif
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index be20eff8f358..e8b6e2438d31 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -533,6 +533,7 @@ static const char *uaccess_safe_builtin[] = {
 	"__ubsan_handle_shift_out_of_bounds",
 	/* misc */
 	"csum_partial_copy_generic",
+	"copy_safe_fast",
 	"copy_safe_slow",
 	"copy_safe_slow_handle_tail",
 	"ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

  parent reply	other threads:[~2020-04-30  8:41 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-30  8:24 [PATCH v2 0/2] Replace and improve "mcsafe" with copy_safe() Dan Williams
2020-04-30  8:25 ` [PATCH v2 1/2] copy_safe: Rename memcpy_mcsafe() to copy_safe() Dan Williams
2020-04-30  8:25 ` Dan Williams [this message]
2020-04-30 14:02 ` [PATCH v2 0/2] Replace and improve "mcsafe" with copy_safe() Linus Torvalds
2020-04-30 16:51   ` Andy Lutomirski
2020-04-30 17:17     ` Linus Torvalds
2020-04-30 18:42       ` Andy Lutomirski
2020-04-30 19:22         ` Luck, Tony
2020-04-30 19:50           ` Linus Torvalds
2020-04-30 20:25             ` Luck, Tony
2020-04-30 23:52             ` Dan Williams
2020-05-01  0:10               ` Linus Torvalds
2020-05-01  0:23                 ` Andy Lutomirski
2020-05-01  0:39                   ` Linus Torvalds
2020-05-01  1:10                     ` Andy Lutomirski
2020-05-01 14:09                   ` Luck, Tony
2020-05-03  0:29                     ` Andy Lutomirski
2020-05-04 20:05                       ` Luck, Tony
2020-05-04 20:26                         ` Andy Lutomirski
2020-05-04 21:30                           ` Dan Williams
2020-05-01  0:24                 ` Linus Torvalds
2020-05-01  1:20                   ` Andy Lutomirski
2020-05-01  1:21                 ` Dan Williams
2020-05-01 18:28                   ` Linus Torvalds
2020-05-01 20:17                     ` Dave Hansen
2020-05-03 12:57                     ` David Laight
2020-05-04 18:33                       ` Dan Williams
2020-05-11 15:24                   ` Vivek Goyal
2020-04-30 19:51           ` Dan Williams
2020-04-30 20:07             ` Andy Lutomirski
2020-05-01  7:46         ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=158823510897.2094061.8955960257425244970.stgit@dwillia2-desk3.amr.corp.intel.com \
    --to=dan.j.williams@intel.com \
    --cc=bp@alien8.de \
    --cc=erwin.tsaur@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).