From: Mark Rutland <mark.rutland@arm.com>
To: Tong Tiangen <tongtiangen@huawei.com>
Cc: James Morse <james.morse@arm.com>,
Andrew Morton <akpm@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Robin Murphy <robin.murphy@arm.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Michael Ellerman <mpe@ellerman.id.au>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Kefeng Wang <wangkefeng.wang@huawei.com>,
Xie XiuQi <xiexiuqi@huawei.com>, Guohanjun <guohanjun@huawei.com>
Subject: Re: [PATCH -next v4 4/7] arm64: add copy_{to, from}_user to machine check safe
Date: Fri, 13 May 2022 16:31:07 +0100 [thread overview]
Message-ID: <Yn55uz52Hn3gF9Xe@lakrids> (raw)
In-Reply-To: <20220420030418.3189040-5-tongtiangen@huawei.com>
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 <tongtiangen@huawei.com>
> ---
> 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
>
next prev parent reply other threads:[~2022-05-13 15:31 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-20 3:04 [PATCH -next v4 0/7]arm64: add machine check safe support Tong Tiangen
2022-04-20 3:04 ` [PATCH -next v4 1/7] x86, powerpc: fix function define in copy_mc_to_user Tong Tiangen
2022-04-22 9:45 ` Michael Ellerman
2022-04-24 1:16 ` Tong Tiangen
2022-05-02 14:24 ` Christophe Leroy
2022-05-03 1:06 ` Tong Tiangen
2022-05-05 1:21 ` Kefeng Wang
2022-04-20 3:04 ` [PATCH -next v4 2/7] arm64: fix types in copy_highpage() Tong Tiangen
2022-04-20 3:04 ` [PATCH -next v4 3/7] arm64: add support for machine check error safe Tong Tiangen
2022-05-13 15:26 ` Mark Rutland
2022-05-19 6:29 ` Tong Tiangen
2022-05-25 8:30 ` Mark Rutland
2022-05-26 3:36 ` Tong Tiangen
2022-05-26 9:50 ` Mark Rutland
2022-05-27 1:40 ` Tong Tiangen
2022-04-20 3:04 ` [PATCH -next v4 4/7] arm64: add copy_{to, from}_user to machine check safe Tong Tiangen
2022-05-04 10:26 ` Catalin Marinas
2022-05-05 6:39 ` Tong Tiangen
2022-05-05 13:41 ` Catalin Marinas
2022-05-05 14:33 ` Tong Tiangen
2022-05-13 15:31 ` Mark Rutland [this message]
2022-05-19 6:53 ` Tong Tiangen
2022-04-20 3:04 ` [PATCH -next v4 5/7] arm64: mte: Clean up user tag accessors Tong Tiangen
2022-05-13 15:36 ` Mark Rutland
2022-04-20 3:04 ` [PATCH -next v4 6/7] arm64: add {get, put}_user to machine check safe Tong Tiangen
2022-05-13 15:39 ` Mark Rutland
2022-05-19 7:09 ` Tong Tiangen
2022-04-20 3:04 ` [PATCH -next v4 7/7] arm64: add cow " Tong Tiangen
2022-05-13 15:44 ` Mark Rutland
2022-05-19 10:38 ` Tong Tiangen
2022-04-27 9:09 ` [PATCH -next v4 0/7]arm64: add machine check safe support Tong Tiangen
2022-05-04 19:58 ` (subset) " Catalin Marinas
2022-05-16 18:45 ` Catalin Marinas
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=Yn55uz52Hn3gF9Xe@lakrids \
--to=mark.rutland@arm.com \
--cc=akpm@linux-foundation.org \
--cc=benh@kernel.crashing.org \
--cc=bp@alien8.de \
--cc=catalin.marinas@arm.com \
--cc=dave.hansen@linux.intel.com \
--cc=guohanjun@huawei.com \
--cc=hpa@zytor.com \
--cc=james.morse@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@redhat.com \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
--cc=robin.murphy@arm.com \
--cc=tglx@linutronix.de \
--cc=tongtiangen@huawei.com \
--cc=viro@zeniv.linux.org.uk \
--cc=wangkefeng.wang@huawei.com \
--cc=will@kernel.org \
--cc=x86@kernel.org \
--cc=xiexiuqi@huawei.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;
as well as URLs for NNTP newsgroup(s).