Archive-only list for patches
 help / color / mirror / Atom feed
From: Tony Luck <tony.luck@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	patches@lists.linux.dev, Youquan Song <youquan.song@intel.com>,
	Tony Luck <tony.luck@intel.com>
Subject: [PATCH] x86/mce: Reduce number of machine checks taken during recovery
Date: Wed, 15 Dec 2021 14:20:16 -0800	[thread overview]
Message-ID: <20211215222016.1390235-1-tony.luck@intel.com> (raw)

From: Youquan Song <youquan.song@intel.com>

A machine check recovery test case injects poison into memory and then
tries a write(2) system call with a buffer that begins 64 bytes before
the poison and includes the first poison byte:

$ sudo ./einj_mem_uc -f -m 64:65:0 copyin
0: copyin   vaddr = 0x7f13639cc400 paddr = 834fa0d400
./einj_mem_uc: short (64 bytes) write to temp file
page not present
Big surprise ... still running. Thought that would be fatal
Unusual number of MCEs seen: 4
Test passed

While it executes correctly (just the 64 non-poison bytes are written)
and recovers, four machine checks seems like a lot.

When any of the copy functions in arch/x86/lib/copy_user_64.S consume
poison and take a fault the fixup code copies the remaining byte count
from %ecx to %edx and unconditionally jumps to .Lcopy_user_handle_tail
to continue the copy in case any more bytes can be copied.

If the fault was #PF this may copy more bytes (because the page fault
handler might have fixed the fault). But when the fault is a machine
check the original copy code will have copied all the way to the poisoned
cache line. So .Lcopy_user_handle_tail will just take another machine
check for no good reason.

There are five distinct blocks of fixup code that branch to
.Lcopy_user_handle_tail, so add a check there to check the trap type
(in %eax) and simply return the count of remaining bytes.

As well as reducing the number of machine checks, this also allows
Skylake generation Xeons to recover some cases that currently fail. The
is because "rep movsb" is only recoverable when source and destination
are well aligned and the byte count is large. That useless call to
.Lcopy_user_handle_tail may violate one or more of these conditions and
generate a fatal machine check.

With this change applied the count of machine checks to recover is
reduced from four to three:

$ sudo ./einj_mem_uc -f -m 64:65:0 copyin
0: copyin   vaddr = 0x7f1e44845400 paddr = 55930b400
./einj_mem_uc: short (64 bytes) write to temp file
page not present
Big surprise ... still running. Thought that would be fatal
Unusual number of MCEs seen: 3
Test passed

[Tony: Added more details to commit message]

Signed-off-by: Youquan Song <youquan.song@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/lib/copy_user_64.S | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
index 2797e630b9b1..8c53be99faa0 100644
--- a/arch/x86/lib/copy_user_64.S
+++ b/arch/x86/lib/copy_user_64.S
@@ -233,12 +233,19 @@ EXPORT_SYMBOL(copy_user_enhanced_fast_string)
  * eax uncopied bytes or 0 if successful.
  */
 SYM_CODE_START_LOCAL(.Lcopy_user_handle_tail)
+	cmp $X86_TRAP_MC,%eax
+	je 3f
 	movl %edx,%ecx
 1:	rep movsb
 2:	mov %ecx,%eax
 	ASM_CLAC
 	ret
 
+3:
+	movl %edx,%eax
+	ASM_CLAC
+	ret
+
 	_ASM_EXTABLE_CPY(1b, 2b)
 SYM_CODE_END(.Lcopy_user_handle_tail)
 
-- 
2.31.1


             reply	other threads:[~2021-12-15 22:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-15 22:20 Tony Luck [this message]
2021-12-18  0:53 ` [PATCH] x86/mce: Reduce number of machine checks taken during recovery Peter Zijlstra
2021-12-23 20:07   ` [PATCH v2] " Luck, Tony
2021-12-24 11:50     ` Peter Zijlstra
2021-12-27 10:52       ` Borislav Petkov

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=20211215222016.1390235-1-tony.luck@intel.com \
    --to=tony.luck@intel.com \
    --cc=bp@alien8.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=x86@kernel.org \
    --cc=youquan.song@intel.com \
    /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