From: Jisheng Zhang <jszhang@kernel.org>
To: Mayuresh Chitale <mchitale@ventanamicro.com>
Cc: "Paul Walmsley" <paul.walmsley@sifive.com>,
"Palmer Dabbelt" <palmer@dabbelt.com>,
"Albert Ou" <aou@eecs.berkeley.edu>,
"Björn Töpel" <bjorn@kernel.org>,
"Alexei Starovoitov" <ast@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Martin KaFai Lau" <kafai@fb.com>,
"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
"John Fastabend" <john.fastabend@gmail.com>,
"KP Singh" <kpsingh@kernel.org>,
"Masahiro Yamada" <masahiroy@kernel.org>,
"Michal Marek" <michal.lkml@markovi.net>,
"Nick Desaulniers" <ndesaulniers@google.com>,
"Kefeng Wang" <wangkefeng.wang@huawei.com>,
"Tong Tiangen" <tongtiangen@huawei.com>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, bpf@vger.kernel.org,
linux-kbuild@vger.kernel.org
Subject: Re: [PATCH 11/12] riscv: extable: add a dedicated uaccess handler
Date: Fri, 21 Jan 2022 20:16:32 +0800 [thread overview]
Message-ID: <YeqkIKUsdHH0ORxf@xhacker> (raw)
In-Reply-To: <CAN37VV6vfee+T18UkbDLe1ts87+Zvg25oQR1+VJD3e6SJFPPiA@mail.gmail.com>
On Thu, Jan 20, 2022 at 11:45:34PM +0530, Mayuresh Chitale wrote:
> Hello Jisheng,
Hi,
>
> Just wanted to inform you that this patch breaks the writev02 test
> case in LTP and if it is reverted then the test passes. If we run the
> test through strace then we see that the test hangs and following is
> the last line printed by strace:
>
> "writev(3, [{iov_base=0x7fff848a6000, iov_len=8192}, {iov_base=NULL,
> iov_len=0}]"
>
Thanks for the bug report. I will try to fix it.
> Thanks,
> Mayuresh.
>
>
> On Thu, Nov 18, 2021 at 5:05 PM Jisheng Zhang <jszhang3@mail.ustc.edu.cn> wrote:
> >
> > From: Jisheng Zhang <jszhang@kernel.org>
> >
> > Inspired by commit 2e77a62cb3a6("arm64: extable: add a dedicated
> > uaccess handler"), do similar to riscv to add a dedicated uaccess
> > exception handler to update registers in exception context and
> > subsequently return back into the function which faulted, so we remove
> > the need for fixups specialized to each faulting instruction.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> > arch/riscv/include/asm/asm-extable.h | 23 +++++++++
> > arch/riscv/include/asm/futex.h | 23 +++------
> > arch/riscv/include/asm/uaccess.h | 74 +++++++++-------------------
> > arch/riscv/mm/extable.c | 27 ++++++++++
> > 4 files changed, 78 insertions(+), 69 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/asm-extable.h b/arch/riscv/include/asm/asm-extable.h
> > index 1b1f4ffd8d37..14be0673f5b5 100644
> > --- a/arch/riscv/include/asm/asm-extable.h
> > +++ b/arch/riscv/include/asm/asm-extable.h
> > @@ -5,6 +5,7 @@
> > #define EX_TYPE_NONE 0
> > #define EX_TYPE_FIXUP 1
> > #define EX_TYPE_BPF 2
> > +#define EX_TYPE_UACCESS_ERR_ZERO 3
> >
> > #ifdef __ASSEMBLY__
> >
> > @@ -23,7 +24,9 @@
> >
> > #else /* __ASSEMBLY__ */
> >
> > +#include <linux/bits.h>
> > #include <linux/stringify.h>
> > +#include <asm/gpr-num.h>
> >
> > #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \
> > ".pushsection __ex_table, \"a\"\n" \
> > @@ -37,6 +40,26 @@
> > #define _ASM_EXTABLE(insn, fixup) \
> > __ASM_EXTABLE_RAW(#insn, #fixup, __stringify(EX_TYPE_FIXUP), "0")
> >
> > +#define EX_DATA_REG_ERR_SHIFT 0
> > +#define EX_DATA_REG_ERR GENMASK(4, 0)
> > +#define EX_DATA_REG_ZERO_SHIFT 5
> > +#define EX_DATA_REG_ZERO GENMASK(9, 5)
> > +
> > +#define EX_DATA_REG(reg, gpr) \
> > + "((.L__gpr_num_" #gpr ") << " __stringify(EX_DATA_REG_##reg##_SHIFT) ")"
> > +
> > +#define _ASM_EXTABLE_UACCESS_ERR_ZERO(insn, fixup, err, zero) \
> > + __DEFINE_ASM_GPR_NUMS \
> > + __ASM_EXTABLE_RAW(#insn, #fixup, \
> > + __stringify(EX_TYPE_UACCESS_ERR_ZERO), \
> > + "(" \
> > + EX_DATA_REG(ERR, err) " | " \
> > + EX_DATA_REG(ZERO, zero) \
> > + ")")
> > +
> > +#define _ASM_EXTABLE_UACCESS_ERR(insn, fixup, err) \
> > + _ASM_EXTABLE_UACCESS_ERR_ZERO(insn, fixup, err, zero)
> > +
> > #endif /* __ASSEMBLY__ */
> >
> > #endif /* __ASM_ASM_EXTABLE_H */
> > diff --git a/arch/riscv/include/asm/futex.h b/arch/riscv/include/asm/futex.h
> > index 2e15e8e89502..fc8130f995c1 100644
> > --- a/arch/riscv/include/asm/futex.h
> > +++ b/arch/riscv/include/asm/futex.h
> > @@ -21,20 +21,14 @@
> >
> > #define __futex_atomic_op(insn, ret, oldval, uaddr, oparg) \
> > { \
> > - uintptr_t tmp; \
> > __enable_user_access(); \
> > __asm__ __volatile__ ( \
> > "1: " insn " \n" \
> > "2: \n" \
> > - " .section .fixup,\"ax\" \n" \
> > - " .balign 4 \n" \
> > - "3: li %[r],%[e] \n" \
> > - " jump 2b,%[t] \n" \
> > - " .previous \n" \
> > - _ASM_EXTABLE(1b, 3b) \
> > + _ASM_EXTABLE_UACCESS_ERR(1b, 2b, %[r]) \
> > : [r] "+r" (ret), [ov] "=&r" (oldval), \
> > - [u] "+m" (*uaddr), [t] "=&r" (tmp) \
> > - : [op] "Jr" (oparg), [e] "i" (-EFAULT) \
> > + [u] "+m" (*uaddr) \
> > + : [op] "Jr" (oparg) \
> > : "memory"); \
> > __disable_user_access(); \
> > }
> > @@ -96,15 +90,10 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
> > "2: sc.w.aqrl %[t],%z[nv],%[u] \n"
> > " bnez %[t],1b \n"
> > "3: \n"
> > - " .section .fixup,\"ax\" \n"
> > - " .balign 4 \n"
> > - "4: li %[r],%[e] \n"
> > - " jump 3b,%[t] \n"
> > - " .previous \n"
> > - _ASM_EXTABLE(1b, 4b) \
> > - _ASM_EXTABLE(2b, 4b) \
> > + _ASM_EXTABLE_UACCESS_ERR(1b, 3b, %[r]) \
> > + _ASM_EXTABLE_UACCESS_ERR(2b, 3b, %[r]) \
> > : [r] "+r" (ret), [v] "=&r" (val), [u] "+m" (*uaddr), [t] "=&r" (tmp)
> > - : [ov] "Jr" (oldval), [nv] "Jr" (newval), [e] "i" (-EFAULT)
> > + : [ov] "Jr" (oldval), [nv] "Jr" (newval)
> > : "memory");
> > __disable_user_access();
> >
> > diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> > index 40e6099af488..a4716c026386 100644
> > --- a/arch/riscv/include/asm/uaccess.h
> > +++ b/arch/riscv/include/asm/uaccess.h
> > @@ -81,22 +81,14 @@ static inline int __access_ok(unsigned long addr, unsigned long size)
> >
> > #define __get_user_asm(insn, x, ptr, err) \
> > do { \
> > - uintptr_t __tmp; \
> > __typeof__(x) __x; \
> > __asm__ __volatile__ ( \
> > "1:\n" \
> > - " " insn " %1, %3\n" \
> > + " " insn " %1, %2\n" \
> > "2:\n" \
> > - " .section .fixup,\"ax\"\n" \
> > - " .balign 4\n" \
> > - "3:\n" \
> > - " li %0, %4\n" \
> > - " li %1, 0\n" \
> > - " jump 2b, %2\n" \
> > - " .previous\n" \
> > - _ASM_EXTABLE(1b, 3b) \
> > - : "+r" (err), "=&r" (__x), "=r" (__tmp) \
> > - : "m" (*(ptr)), "i" (-EFAULT)); \
> > + _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 2b, %0, %1) \
> > + : "+r" (err), "=&r" (__x) \
> > + : "m" (*(ptr))); \
> > (x) = __x; \
> > } while (0)
> >
> > @@ -108,27 +100,18 @@ do { \
> > do { \
> > u32 __user *__ptr = (u32 __user *)(ptr); \
> > u32 __lo, __hi; \
> > - uintptr_t __tmp; \
> > __asm__ __volatile__ ( \
> > "1:\n" \
> > - " lw %1, %4\n" \
> > + " lw %1, %3\n" \
> > "2:\n" \
> > - " lw %2, %5\n" \
> > + " lw %2, %4\n" \
> > "3:\n" \
> > - " .section .fixup,\"ax\"\n" \
> > - " .balign 4\n" \
> > - "4:\n" \
> > - " li %0, %6\n" \
> > - " li %1, 0\n" \
> > - " li %2, 0\n" \
> > - " jump 3b, %3\n" \
> > - " .previous\n" \
> > - _ASM_EXTABLE(1b, 4b) \
> > - _ASM_EXTABLE(2b, 4b) \
> > - : "+r" (err), "=&r" (__lo), "=r" (__hi), \
> > - "=r" (__tmp) \
> > - : "m" (__ptr[__LSW]), "m" (__ptr[__MSW]), \
> > - "i" (-EFAULT)); \
> > + _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 3b, %0, %1) \
> > + _ASM_EXTABLE_UACCESS_ERR_ZERO(2b, 3b, %0, %1) \
> > + : "+r" (err), "=&r" (__lo), "=r" (__hi) \
> > + : "m" (__ptr[__LSW]), "m" (__ptr[__MSW])) \
> > + if (err) \
> > + __hi = 0; \
> > (x) = (__typeof__(x))((__typeof__((x)-(x)))( \
> > (((u64)__hi << 32) | __lo))); \
> > } while (0)
> > @@ -216,21 +199,14 @@ do { \
> >
> > #define __put_user_asm(insn, x, ptr, err) \
> > do { \
> > - uintptr_t __tmp; \
> > __typeof__(*(ptr)) __x = x; \
> > __asm__ __volatile__ ( \
> > "1:\n" \
> > - " " insn " %z3, %2\n" \
> > + " " insn " %z2, %1\n" \
> > "2:\n" \
> > - " .section .fixup,\"ax\"\n" \
> > - " .balign 4\n" \
> > - "3:\n" \
> > - " li %0, %4\n" \
> > - " jump 2b, %1\n" \
> > - " .previous\n" \
> > - _ASM_EXTABLE(1b, 3b) \
> > - : "+r" (err), "=r" (__tmp), "=m" (*(ptr)) \
> > - : "rJ" (__x), "i" (-EFAULT)); \
> > + _ASM_EXTABLE_UACCESS_ERR(1b, 2b, %0) \
> > + : "+r" (err), "=m" (*(ptr)) \
> > + : "rJ" (__x)); \
> > } while (0)
> >
> > #ifdef CONFIG_64BIT
> > @@ -244,22 +220,16 @@ do { \
> > uintptr_t __tmp; \
> > __asm__ __volatile__ ( \
> > "1:\n" \
> > - " sw %z4, %2\n" \
> > + " sw %z3, %1\n" \
> > "2:\n" \
> > - " sw %z5, %3\n" \
> > + " sw %z4, %2\n" \
> > "3:\n" \
> > - " .section .fixup,\"ax\"\n" \
> > - " .balign 4\n" \
> > - "4:\n" \
> > - " li %0, %6\n" \
> > - " jump 3b, %1\n" \
> > - " .previous\n" \
> > - _ASM_EXTABLE(1b, 4b) \
> > - _ASM_EXTABLE(2b, 4b) \
> > - : "+r" (err), "=r" (__tmp), \
> > + _ASM_EXTABLE_UACCESS_ERR(1b, 3b, %0) \
> > + _ASM_EXTABLE_UACCESS_ERR(2b, 3b, %0) \
> > + : "+r" (err), \
> > "=m" (__ptr[__LSW]), \
> > "=m" (__ptr[__MSW]) \
> > - : "rJ" (__x), "rJ" (__x >> 32), "i" (-EFAULT)); \
> > + : "rJ" (__x), "rJ" (__x >> 32)); \
> > } while (0)
> > #endif /* CONFIG_64BIT */
> >
> > diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
> > index 91e52c4bb33a..05978f78579f 100644
> > --- a/arch/riscv/mm/extable.c
> > +++ b/arch/riscv/mm/extable.c
> > @@ -7,10 +7,12 @@
> > */
> >
> >
> > +#include <linux/bitfield.h>
> > #include <linux/extable.h>
> > #include <linux/module.h>
> > #include <linux/uaccess.h>
> > #include <asm/asm-extable.h>
> > +#include <asm/ptrace.h>
> >
> > static inline unsigned long
> > get_ex_fixup(const struct exception_table_entry *ex)
> > @@ -25,6 +27,29 @@ static bool ex_handler_fixup(const struct exception_table_entry *ex,
> > return true;
> > }
> >
> > +static inline void regs_set_gpr(struct pt_regs *regs, unsigned int offset,
> > + unsigned long val)
> > +{
> > + if (unlikely(offset > MAX_REG_OFFSET))
> > + return;
> > +
> > + if (!offset)
> > + *(unsigned long *)((unsigned long)regs + offset) = val;
> > +}
> > +
> > +static bool ex_handler_uaccess_err_zero(const struct exception_table_entry *ex,
> > + struct pt_regs *regs)
> > +{
> > + int reg_err = FIELD_GET(EX_DATA_REG_ERR, ex->data);
> > + int reg_zero = FIELD_GET(EX_DATA_REG_ZERO, ex->data);
> > +
> > + regs_set_gpr(regs, reg_err, -EFAULT);
> > + regs_set_gpr(regs, reg_zero, 0);
> > +
> > + regs->epc = get_ex_fixup(ex);
> > + return true;
> > +}
> > +
> > bool fixup_exception(struct pt_regs *regs)
> > {
> > const struct exception_table_entry *ex;
> > @@ -38,6 +63,8 @@ bool fixup_exception(struct pt_regs *regs)
> > return ex_handler_fixup(ex, regs);
> > case EX_TYPE_BPF:
> > return ex_handler_bpf(ex, regs);
> > + case EX_TYPE_UACCESS_ERR_ZERO:
> > + return ex_handler_uaccess_err_zero(ex, regs);
> > }
> >
> > BUG();
> > --
> > 2.33.0
> >
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2022-01-21 12:24 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-18 11:21 [PATCH v4 0/12] riscv: switch to relative extable and other improvements Jisheng Zhang
2021-11-18 11:22 ` [PATCH 01/12] riscv: remove unused __cmpxchg_user() macro Jisheng Zhang
2021-11-18 11:22 ` [PATCH 02/12] riscv: consolidate __ex_table construction Jisheng Zhang
2021-11-18 11:22 ` [PATCH 03/12] riscv: switch to relative exception tables Jisheng Zhang
2021-11-19 2:09 ` tongtiangen
2021-11-18 11:23 ` [PATCH 04/12] riscv: bpf: move rv_bpf_fixup_exception signature to extable.h Jisheng Zhang
2021-11-18 11:24 ` [PATCH 05/12] riscv: extable: make fixup_exception() return bool Jisheng Zhang
2021-11-18 11:24 ` [PATCH 06/12] riscv: extable: use `ex` for `exception_table_entry` Jisheng Zhang
2021-11-18 11:25 ` [PATCH 07/12] riscv: lib: uaccess: fold fixups into body Jisheng Zhang
2021-11-18 11:25 ` [PATCH 08/12] riscv: extable: consolidate definitions Jisheng Zhang
2021-11-18 11:26 ` [PATCH 09/12] riscv: extable: add `type` and `data` fields Jisheng Zhang
2021-11-18 11:42 ` Jisheng Zhang
2021-11-18 15:21 ` Mark Rutland
2022-01-06 3:21 ` Palmer Dabbelt
2022-01-06 10:23 ` Mark Rutland
2021-11-19 2:35 ` tongtiangen
2021-11-18 11:26 ` [PATCH 10/12] riscv: add gpr-num.h Jisheng Zhang
2021-11-18 11:26 ` [PATCH 11/12] riscv: extable: add a dedicated uaccess handler Jisheng Zhang
2022-01-20 18:15 ` Mayuresh Chitale
2022-01-21 12:16 ` Jisheng Zhang [this message]
2022-01-23 9:12 ` Jisheng Zhang
2022-01-24 17:02 ` Mayuresh Chitale
2021-11-18 11:27 ` [PATCH 12/12] riscv: vmlinux.lds.S|vmlinux-xip.lds.S: remove `.fixup` section Jisheng Zhang
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=YeqkIKUsdHH0ORxf@xhacker \
--to=jszhang@kernel.org \
--cc=andrii@kernel.org \
--cc=aou@eecs.berkeley.edu \
--cc=ast@kernel.org \
--cc=bjorn@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=masahiroy@kernel.org \
--cc=mchitale@ventanamicro.com \
--cc=michal.lkml@markovi.net \
--cc=ndesaulniers@google.com \
--cc=netdev@vger.kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=songliubraving@fb.com \
--cc=tongtiangen@huawei.com \
--cc=wangkefeng.wang@huawei.com \
--cc=yhs@fb.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).