From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF9E5C433EF for ; Fri, 13 May 2022 15:31:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4C7F36B0073; Fri, 13 May 2022 11:31:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 477C38D0002; Fri, 13 May 2022 11:31:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 33E548D0001; Fri, 13 May 2022 11:31:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 276F06B0073 for ; Fri, 13 May 2022 11:31:19 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E6330E87 for ; Fri, 13 May 2022 15:31:18 +0000 (UTC) X-FDA: 79461108636.09.53993F3 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf14.hostedemail.com (Postfix) with ESMTP id DE9C61000B1 for ; Fri, 13 May 2022 15:31:15 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5BB171424; Fri, 13 May 2022 08:31:17 -0700 (PDT) Received: from lakrids (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8E3743F73D; Fri, 13 May 2022 08:31:14 -0700 (PDT) Date: Fri, 13 May 2022 16:31:07 +0100 From: Mark Rutland To: Tong Tiangen Cc: James Morse , Andrew Morton , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Robin Murphy , Dave Hansen , Catalin Marinas , Will Deacon , Alexander Viro , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , x86@kernel.org, "H . Peter Anvin" , linuxppc-dev@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Kefeng Wang , Xie XiuQi , Guohanjun Subject: Re: [PATCH -next v4 4/7] arm64: add copy_{to, from}_user to machine check safe Message-ID: References: <20220420030418.3189040-1-tongtiangen@huawei.com> <20220420030418.3189040-5-tongtiangen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220420030418.3189040-5-tongtiangen@huawei.com> X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: DE9C61000B1 Authentication-Results: imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of mark.rutland@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=mark.rutland@arm.com; dmarc=pass (policy=none) header.from=arm.com X-Rspam-User: X-Stat-Signature: ow3of61rwpeappb5o5ohrjakmkekpja7 X-HE-Tag: 1652455875-490892 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Apr 20, 2022 at 03:04:15AM +0000, Tong Tiangen wrote: > Add copy_{to, from}_user() to machine check safe. > > If copy fail due to hardware memory error, only the relevant processes are > affected, so killing the user process and isolate the user page with > hardware memory errors is a more reasonable choice than kernel panic. > > Add new extable type EX_TYPE_UACCESS_MC which can be used for uaccess that > can be recovered from hardware memory errors. I don't understand why we need this. If we apply EX_TYPE_UACCESS consistently to *all* user accesses, and *only* to user accesses, that would *always* indicate that we can recover, and that seems much simpler to deal with. Today we use EX_TYPE_UACCESS_ERR_ZERO for kernel accesses in a couple of cases, which we should clean up, and we user EX_TYPE_FIXUP for a couple of user accesses, but those could easily be converted over. > The x16 register is used to save the fixup type in copy_xxx_user which > used extable type EX_TYPE_UACCESS_MC. Why x16? How is this intended to be consumed, and why is that behaviour different from any *other* fault? Mark. > Signed-off-by: Tong Tiangen > --- > arch/arm64/include/asm/asm-extable.h | 14 ++++++++++++++ > arch/arm64/include/asm/asm-uaccess.h | 15 ++++++++++----- > arch/arm64/lib/copy_from_user.S | 18 +++++++++++------- > arch/arm64/lib/copy_to_user.S | 18 +++++++++++------- > arch/arm64/mm/extable.c | 18 ++++++++++++++---- > 5 files changed, 60 insertions(+), 23 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h > index c39f2437e08e..75b2c00e9523 100644 > --- a/arch/arm64/include/asm/asm-extable.h > +++ b/arch/arm64/include/asm/asm-extable.h > @@ -2,12 +2,18 @@ > #ifndef __ASM_ASM_EXTABLE_H > #define __ASM_ASM_EXTABLE_H > > +#define FIXUP_TYPE_NORMAL 0 > +#define FIXUP_TYPE_MC 1 > + > #define EX_TYPE_NONE 0 > #define EX_TYPE_FIXUP 1 > #define EX_TYPE_BPF 2 > #define EX_TYPE_UACCESS_ERR_ZERO 3 > #define EX_TYPE_LOAD_UNALIGNED_ZEROPAD 4 > > +/* _MC indicates that can fixup from machine check errors */ > +#define EX_TYPE_UACCESS_MC 5 > + > #ifdef __ASSEMBLY__ > > #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ > @@ -27,6 +33,14 @@ > __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) > .endm > > +/* > + * Create an exception table entry for `insn`, which will branch to `fixup` > + * when an unhandled fault(include sea fault) is taken. > + */ > + .macro _asm_extable_uaccess_mc, insn, fixup > + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_MC, 0) > + .endm > + > /* > * Create an exception table entry for `insn` if `fixup` is provided. Otherwise > * do nothing. > diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h > index 0557af834e03..6c23c138e1fc 100644 > --- a/arch/arm64/include/asm/asm-uaccess.h > +++ b/arch/arm64/include/asm/asm-uaccess.h > @@ -63,6 +63,11 @@ alternative_else_nop_endif > 9999: x; \ > _asm_extable 9999b, l > > + > +#define USER_MC(l, x...) \ > +9999: x; \ > + _asm_extable_uaccess_mc 9999b, l > + > /* > * Generate the assembly for LDTR/STTR with exception table entries. > * This is complicated as there is no post-increment or pair versions of the > @@ -73,8 +78,8 @@ alternative_else_nop_endif > 8889: ldtr \reg2, [\addr, #8]; > add \addr, \addr, \post_inc; > > - _asm_extable 8888b,\l; > - _asm_extable 8889b,\l; > + _asm_extable_uaccess_mc 8888b, \l; > + _asm_extable_uaccess_mc 8889b, \l; > .endm > > .macro user_stp l, reg1, reg2, addr, post_inc > @@ -82,14 +87,14 @@ alternative_else_nop_endif > 8889: sttr \reg2, [\addr, #8]; > add \addr, \addr, \post_inc; > > - _asm_extable 8888b,\l; > - _asm_extable 8889b,\l; > + _asm_extable_uaccess_mc 8888b,\l; > + _asm_extable_uaccess_mc 8889b,\l; > .endm > > .macro user_ldst l, inst, reg, addr, post_inc > 8888: \inst \reg, [\addr]; > add \addr, \addr, \post_inc; > > - _asm_extable 8888b,\l; > + _asm_extable_uaccess_mc 8888b, \l; > .endm > #endif > diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S > index 34e317907524..480cc5ac0a8d 100644 > --- a/arch/arm64/lib/copy_from_user.S > +++ b/arch/arm64/lib/copy_from_user.S > @@ -25,7 +25,7 @@ > .endm > > .macro strb1 reg, ptr, val > - strb \reg, [\ptr], \val > + USER_MC(9998f, strb \reg, [\ptr], \val) > .endm > > .macro ldrh1 reg, ptr, val > @@ -33,7 +33,7 @@ > .endm > > .macro strh1 reg, ptr, val > - strh \reg, [\ptr], \val > + USER_MC(9998f, strh \reg, [\ptr], \val) > .endm > > .macro ldr1 reg, ptr, val > @@ -41,7 +41,7 @@ > .endm > > .macro str1 reg, ptr, val > - str \reg, [\ptr], \val > + USER_MC(9998f, str \reg, [\ptr], \val) > .endm > > .macro ldp1 reg1, reg2, ptr, val > @@ -49,11 +49,12 @@ > .endm > > .macro stp1 reg1, reg2, ptr, val > - stp \reg1, \reg2, [\ptr], \val > + USER_MC(9998f, stp \reg1, \reg2, [\ptr], \val) > .endm > > -end .req x5 > -srcin .req x15 > +end .req x5 > +srcin .req x15 > +fixup_type .req x16 > SYM_FUNC_START(__arch_copy_from_user) > add end, x0, x2 > mov srcin, x1 > @@ -62,7 +63,10 @@ SYM_FUNC_START(__arch_copy_from_user) > ret > > // Exception fixups > -9997: cmp dst, dstin > + // x16: fixup type written by ex_handler_uaccess_mc > +9997: cmp fixup_type, #FIXUP_TYPE_MC > + b.eq 9998f > + cmp dst, dstin > b.ne 9998f > // Before being absolutely sure we couldn't copy anything, try harder > USER(9998f, ldtrb tmp1w, [srcin]) > diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S > index 802231772608..021a7d27b3a4 100644 > --- a/arch/arm64/lib/copy_to_user.S > +++ b/arch/arm64/lib/copy_to_user.S > @@ -20,7 +20,7 @@ > * x0 - bytes not copied > */ > .macro ldrb1 reg, ptr, val > - ldrb \reg, [\ptr], \val > + USER_MC(9998f, ldrb \reg, [\ptr], \val) > .endm > > .macro strb1 reg, ptr, val > @@ -28,7 +28,7 @@ > .endm > > .macro ldrh1 reg, ptr, val > - ldrh \reg, [\ptr], \val > + USER_MC(9998f, ldrh \reg, [\ptr], \val) > .endm > > .macro strh1 reg, ptr, val > @@ -36,7 +36,7 @@ > .endm > > .macro ldr1 reg, ptr, val > - ldr \reg, [\ptr], \val > + USER_MC(9998f, ldr \reg, [\ptr], \val) > .endm > > .macro str1 reg, ptr, val > @@ -44,15 +44,16 @@ > .endm > > .macro ldp1 reg1, reg2, ptr, val > - ldp \reg1, \reg2, [\ptr], \val > + USER_MC(9998f, ldp \reg1, \reg2, [\ptr], \val) > .endm > > .macro stp1 reg1, reg2, ptr, val > user_stp 9997f, \reg1, \reg2, \ptr, \val > .endm > > -end .req x5 > -srcin .req x15 > +end .req x5 > +srcin .req x15 > +fixup_type .req x16 > SYM_FUNC_START(__arch_copy_to_user) > add end, x0, x2 > mov srcin, x1 > @@ -61,7 +62,10 @@ SYM_FUNC_START(__arch_copy_to_user) > ret > > // Exception fixups > -9997: cmp dst, dstin > + // x16: fixup type written by ex_handler_uaccess_mc > +9997: cmp fixup_type, #FIXUP_TYPE_MC > + b.eq 9998f > + cmp dst, dstin > b.ne 9998f > // Before being absolutely sure we couldn't copy anything, try harder > ldrb tmp1w, [srcin] > diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c > index 4f0083a550d4..525876c3ebf4 100644 > --- a/arch/arm64/mm/extable.c > +++ b/arch/arm64/mm/extable.c > @@ -24,6 +24,14 @@ static bool ex_handler_fixup(const struct exception_table_entry *ex, > return true; > } > > +static bool ex_handler_uaccess_type(const struct exception_table_entry *ex, > + struct pt_regs *regs, > + unsigned long fixup_type) > +{ > + regs->regs[16] = fixup_type; > + return ex_handler_fixup(ex, regs); > +} > + > static bool ex_handler_uaccess_err_zero(const struct exception_table_entry *ex, > struct pt_regs *regs) > { > @@ -75,6 +83,8 @@ bool fixup_exception(struct pt_regs *regs) > switch (ex->type) { > case EX_TYPE_FIXUP: > return ex_handler_fixup(ex, regs); > + case EX_TYPE_UACCESS_MC: > + return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_NORMAL); > case EX_TYPE_BPF: > return ex_handler_bpf(ex, regs); > case EX_TYPE_UACCESS_ERR_ZERO: > @@ -94,10 +104,10 @@ bool fixup_exception_mc(struct pt_regs *regs) > if (!ex) > return false; > > - /* > - * This is not complete, More Machine check safe extable type can > - * be processed here. > - */ > + switch (ex->type) { > + case EX_TYPE_UACCESS_MC: > + return ex_handler_uaccess_type(ex, regs, FIXUP_TYPE_MC); > + } > > return false; > } > -- > 2.25.1 >