Message ID | 20230622173613.30722-1-bjorn@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp5248701vqr; Thu, 22 Jun 2023 11:05:51 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5RtRcV5QRZwAXqn6n5CgK31COMAna92zUEyBJ0vdBCdmFD4CcSFDkoMpHsHJPKV32wIgs0 X-Received: by 2002:a17:903:25c9:b0:1b3:e6ba:1573 with SMTP id jc9-20020a17090325c900b001b3e6ba1573mr12593391plb.6.1687457151039; Thu, 22 Jun 2023 11:05:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687457151; cv=none; d=google.com; s=arc-20160816; b=uJFRKGGKtKqRIgaKFASK1n65yD3MTpIbjN+MwZlCdH9pW+ZbhpjC+sq+Ck3TLHYPBY lyHfYmpd44++zbHaldmUwTRzB7bEAu9Lt6fZlC25Luz5eBQnkMoOlRT0YH+s2S/C7W+1 wWSLdwMUy1aUGTcqNmfvJvaiQYr3KOY2gRp6+91uHhsVTHvhVKVcvC3Z4AWARO8DFPTc Hx6ZsUTAdGptoUhrr+RGPU95OnJRTlEqpJSvBDOsAl9B4BPlZwJLZMPrUfW0qh4jrJke WXHcQ1ioAPFUqYu8NvSMHUmjpy2wDvk+2dUrVH50Ndigl7ODLDxNGjF1Dis5t/dXalGD HUeQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=vu1pnrDa/OJyW+Ohv1bN9I9JtFHiQ/W7U7u2L5MCIvw=; b=xoOfqCIX3/HkeTwtMUv0YKXOdmdtZDwoE6qVa5lWHJqqlrMHh7CJQHAdB+FFse3MHR 9RePaNA87VcoXTYB/YM3qLRyWWWeHyD2O3Nb2yEkbBFPs8+OgOi+dGGlf0MGVHIf2lOm IqMABRObFI4j2FAtuoCUYTjmutZ67jvuVZxFi8ud9IO+IAVSuRw+6hf0PlAicXEGdCZ+ EC5pqyCX4Cxn3OVE3h8POTpqFkJKgIjcInPA5MT80ivatGaXPASlRBon/7txOSMSJcXS IKQjrgRkNI3C0DOcgBVizTrys0+Pa81ZaPNVvBotdBhRd5TkCSBh2x77eRkivrvrsbMz 7yaA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b="SDP/87hu"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id a13-20020a1709027d8d00b001b34159f341si6712282plm.596.2023.06.22.11.05.36; Thu, 22 Jun 2023 11:05:51 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b="SDP/87hu"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231245AbjFVRgZ (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Thu, 22 Jun 2023 13:36:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33882 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229676AbjFVRgX (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 22 Jun 2023 13:36:23 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B1A12195 for <linux-kernel@vger.kernel.org>; Thu, 22 Jun 2023 10:36:22 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 45F3A618BF for <linux-kernel@vger.kernel.org>; Thu, 22 Jun 2023 17:36:22 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 46613C433C0; Thu, 22 Jun 2023 17:36:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1687455381; bh=/OtlEWvMky8FwHoyDb4NvG0us2YcJyx1FBYe9wivhIo=; h=From:To:Cc:Subject:Date:From; b=SDP/87huKee/Zv1UWW27EOV4jrMoXFTtOyzBCCVV2HyEgaNUlEEUByq9LhmSB4cQK akKVNj+YyNFLugitTB+lUVjf2avCXVwEpZeOLECHKo91RYsbYxvVrgshCGdYAZSGn6 Pcbf/v/6u2sZGEpbffUiKeA3Vux7ZXH88rKG/+xBRKsxo3gfRv9pmhjl3KaRCQ7GGB GT+jJOmenD9WBi2fLUfivHK5Jupp7GHhbEDJnkSS73TJVm4wegD6JxV9lNB/a+XeeA iFHcQOnUsZswGhaDS+pRpwrShuald9jYkTRJ4R65G/STQ1uWc06QiHga7mojE0upT+ VoQDDDWWKH93g== From: =?utf-8?b?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org> To: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, linux-riscv@lists.infradead.org Cc: =?utf-8?b?QmrDtnJuIFTDtnBlbA==?= <bjorn@rivosinc.com>, linux-kernel@vger.kernel.org, linux@rivosinc.com, Palmer Dabbelt <palmer@rivosinc.com>, =?utf-8?q?R=C3=A9mi_Denis-Courmont?= <remi@remlab.net>, Darius Rad <darius@bluespec.com>, Andy Chiu <andy.chiu@sifive.com> Subject: [PATCH] riscv: Discard vector state on syscalls Date: Thu, 22 Jun 2023 19:36:13 +0200 Message-Id: <20230622173613.30722-1-bjorn@kernel.org> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769427069733835730?= X-GMAIL-MSGID: =?utf-8?q?1769427069733835730?= |
Series |
riscv: Discard vector state on syscalls
|
|
Commit Message
Björn Töpel
June 22, 2023, 5:36 p.m. UTC
From: Björn Töpel <bjorn@rivosinc.com> The RISC-V vector specification states: Executing a system call causes all caller-saved vector registers (v0-v31, vl, vtype) and vstart to become unspecified. The vector registers are cleared, vill is set (invalid), and the vector status is set to Initial. That way we can prevent userspace from accidentally relying on the stated save. Rémi pointed out [1] that clearing the registers might be superfluous, and setting vill is sufficient. Link: https://lore.kernel.org/linux-riscv/12784326.9UPPK3MAeB@basile.remlab.net/ # [1] Suggested-by: Palmer Dabbelt <palmer@rivosinc.com> Suggested-by: Rémi Denis-Courmont <remi@remlab.net> Signed-off-by: Björn Töpel <bjorn@rivosinc.com> --- I figured I'd sent out a proper patch. I like Andy's optimization patch, but TBH I think we should do that as a follow up. As Rémi pointed out, the clearing might be opted out, but I left it in here. Björn --- arch/riscv/include/asm/vector.h | 25 +++++++++++++++++++++++++ arch/riscv/kernel/traps.c | 2 ++ 2 files changed, 27 insertions(+) base-commit: 4681dacadeefa5ca6017e00736adc1d7dc963c6a
Comments
On Thu, 22 Jun 2023 10:36:13 PDT (-0700), bjorn@kernel.org wrote: > From: Björn Töpel <bjorn@rivosinc.com> > > The RISC-V vector specification states: > Executing a system call causes all caller-saved vector registers > (v0-v31, vl, vtype) and vstart to become unspecified. > > The vector registers are cleared, vill is set (invalid), and the > vector status is set to Initial. > > That way we can prevent userspace from accidentally relying on the > stated save. > > Rémi pointed out [1] that clearing the registers might be superfluous, > and setting vill is sufficient. > > Link: https://lore.kernel.org/linux-riscv/12784326.9UPPK3MAeB@basile.remlab.net/ # [1] > Suggested-by: Palmer Dabbelt <palmer@rivosinc.com> > Suggested-by: Rémi Denis-Courmont <remi@remlab.net> > Signed-off-by: Björn Töpel <bjorn@rivosinc.com> > --- > > I figured I'd sent out a proper patch. I like Andy's optimization > patch, but TBH I think we should do that as a follow up. > > As Rémi pointed out, the clearing might be opted out, but I left it in > here. I think we're going to end up with a bunch of uarch-specific stuff here, but for now having the heavy hammer seems safest. There's no V hardware yet so we can't really tell how anything performs, at least this way we're definately not leaking anything. So I'm OK with this. I'd also be fine with clearing to all-1s, I think it's kind of splitting hairs at this point: the 1s are nice because they're what the rest of V does, but setting vill should make everything trap anyway so maybe it doesn't matter -- and it's not clear if 1 or 0 will allow initial, so who knows. Darius: I'm cool swapping over to the 1s if you feel strongly about it. Bjorn says Sweeden is on vacation, so just LMK and I'll re-spin it with the 1s. Regardless I'd like to pick up something that blows away V state for this merge window, as it'll make sure the uABI is quite strictly enforced. > Björn > > --- > arch/riscv/include/asm/vector.h | 25 +++++++++++++++++++++++++ > arch/riscv/kernel/traps.c | 2 ++ > 2 files changed, 27 insertions(+) > > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > index 04c0b07bf6cd..692ce55e4a69 100644 > --- a/arch/riscv/include/asm/vector.h > +++ b/arch/riscv/include/asm/vector.h > @@ -163,6 +163,30 @@ static inline void __switch_to_vector(struct task_struct *prev, > void riscv_v_vstate_ctrl_init(struct task_struct *tsk); > bool riscv_v_vstate_ctrl_user_allowed(void); > > +static inline void riscv_v_vstate_discard(struct pt_regs *regs) > +{ > + unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1); > + > + if (!riscv_v_vstate_query(regs)) > + return; > + > + riscv_v_enable(); > + asm volatile ( > + ".option push\n\t" > + ".option arch, +v\n\t" > + "vsetvli %0, x0, e8, m8, ta, ma\n\t" > + "vmv.v.i v0, 0\n\t" > + "vmv.v.i v8, 0\n\t" > + "vmv.v.i v16, 0\n\t" > + "vmv.v.i v24, 0\n\t" > + "vsetvl %0, x0, %1\n\t" > + ".option pop\n\t" > + : "=&r" (vl) : "r" (vtype_inval) : "memory"); > + riscv_v_disable(); > + > + riscv_v_vstate_on(regs); > +} > + > #else /* ! CONFIG_RISCV_ISA_V */ > > struct pt_regs; > @@ -178,6 +202,7 @@ static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; } > #define __switch_to_vector(__prev, __next) do {} while (0) > #define riscv_v_vstate_off(regs) do {} while (0) > #define riscv_v_vstate_on(regs) do {} while (0) > +#define riscv_v_vstate_discard(regs) do {} while (0) > > #endif /* CONFIG_RISCV_ISA_V */ > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 05ffdcd1424e..00c68b57ff88 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -295,6 +295,8 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs) > regs->epc += 4; > regs->orig_a0 = regs->a0; > > + riscv_v_vstate_discard(regs); > + > syscall = syscall_enter_from_user_mode(regs, syscall); > > if (syscall < NR_syscalls) > > base-commit: 4681dacadeefa5ca6017e00736adc1d7dc963c6a
On Thu, Jun 22, 2023 at 10:46:31AM -0700, Palmer Dabbelt wrote: > On Thu, 22 Jun 2023 10:36:13 PDT (-0700), bjorn@kernel.org wrote: > > From: Björn Töpel <bjorn@rivosinc.com> > > > > The RISC-V vector specification states: > > Executing a system call causes all caller-saved vector registers > > (v0-v31, vl, vtype) and vstart to become unspecified. > > > > The vector registers are cleared, vill is set (invalid), and the > > vector status is set to Initial. > > > > That way we can prevent userspace from accidentally relying on the > > stated save. > > > > Rémi pointed out [1] that clearing the registers might be superfluous, > > and setting vill is sufficient. > > > > Link: https://lore.kernel.org/linux-riscv/12784326.9UPPK3MAeB@basile.remlab.net/ # [1] > > Suggested-by: Palmer Dabbelt <palmer@rivosinc.com> > > Suggested-by: Rémi Denis-Courmont <remi@remlab.net> > > Signed-off-by: Björn Töpel <bjorn@rivosinc.com> > > --- > > > > I figured I'd sent out a proper patch. I like Andy's optimization > > patch, but TBH I think we should do that as a follow up. > > > > As Rémi pointed out, the clearing might be opted out, but I left it in > > here. > > I think we're going to end up with a bunch of uarch-specific stuff here, but > for now having the heavy hammer seems safest. There's no V hardware yet so > we can't really tell how anything performs, at least this way we're > definately not leaking anything. > > So I'm OK with this. I'd also be fine with clearing to all-1s, I think it's > kind of splitting hairs at this point: the 1s are nice because they're what > the rest of V does, but setting vill should make everything trap anyway so > maybe it doesn't matter -- and it's not clear if 1 or 0 will allow initial, > so who knows. > > Darius: I'm cool swapping over to the 1s if you feel strongly about it. > Bjorn says Sweeden is on vacation, so just LMK and I'll re-spin it with the > 1s. I think all 1s would be preferred, but I don't think it's particularly critical (splitting hairs, like you said), so I'll let you make the call. > > Regardless I'd like to pick up something that blows away V state for this > merge window, as it'll make sure the uABI is quite strictly enforced. > > > Björn > > > > --- > > arch/riscv/include/asm/vector.h | 25 +++++++++++++++++++++++++ > > arch/riscv/kernel/traps.c | 2 ++ > > 2 files changed, 27 insertions(+) > > > > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > > index 04c0b07bf6cd..692ce55e4a69 100644 > > --- a/arch/riscv/include/asm/vector.h > > +++ b/arch/riscv/include/asm/vector.h > > @@ -163,6 +163,30 @@ static inline void __switch_to_vector(struct task_struct *prev, > > void riscv_v_vstate_ctrl_init(struct task_struct *tsk); > > bool riscv_v_vstate_ctrl_user_allowed(void); > > > > +static inline void riscv_v_vstate_discard(struct pt_regs *regs) > > +{ > > + unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1); > > + > > + if (!riscv_v_vstate_query(regs)) > > + return; > > + > > + riscv_v_enable(); > > + asm volatile ( > > + ".option push\n\t" > > + ".option arch, +v\n\t" > > + "vsetvli %0, x0, e8, m8, ta, ma\n\t" > > + "vmv.v.i v0, 0\n\t" > > + "vmv.v.i v8, 0\n\t" > > + "vmv.v.i v16, 0\n\t" > > + "vmv.v.i v24, 0\n\t" > > + "vsetvl %0, x0, %1\n\t" > > + ".option pop\n\t" > > + : "=&r" (vl) : "r" (vtype_inval) : "memory"); > > + riscv_v_disable(); > > + > > + riscv_v_vstate_on(regs); > > +} > > + > > #else /* ! CONFIG_RISCV_ISA_V */ > > > > struct pt_regs; > > @@ -178,6 +202,7 @@ static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; } > > #define __switch_to_vector(__prev, __next) do {} while (0) > > #define riscv_v_vstate_off(regs) do {} while (0) > > #define riscv_v_vstate_on(regs) do {} while (0) > > +#define riscv_v_vstate_discard(regs) do {} while (0) > > > > #endif /* CONFIG_RISCV_ISA_V */ > > > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > > index 05ffdcd1424e..00c68b57ff88 100644 > > --- a/arch/riscv/kernel/traps.c > > +++ b/arch/riscv/kernel/traps.c > > @@ -295,6 +295,8 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs) > > regs->epc += 4; > > regs->orig_a0 = regs->a0; > > > > + riscv_v_vstate_discard(regs); > > + > > syscall = syscall_enter_from_user_mode(regs, syscall); > > > > if (syscall < NR_syscalls) > > > > base-commit: 4681dacadeefa5ca6017e00736adc1d7dc963c6a
On Thu, 22 Jun 2023 10:55:54 PDT (-0700), Darius Rad wrote: > On Thu, Jun 22, 2023 at 10:46:31AM -0700, Palmer Dabbelt wrote: >> On Thu, 22 Jun 2023 10:36:13 PDT (-0700), bjorn@kernel.org wrote: >> > From: Björn Töpel <bjorn@rivosinc.com> >> > >> > The RISC-V vector specification states: >> > Executing a system call causes all caller-saved vector registers >> > (v0-v31, vl, vtype) and vstart to become unspecified. >> > >> > The vector registers are cleared, vill is set (invalid), and the >> > vector status is set to Initial. >> > >> > That way we can prevent userspace from accidentally relying on the >> > stated save. >> > >> > Rémi pointed out [1] that clearing the registers might be superfluous, >> > and setting vill is sufficient. >> > >> > Link: https://lore.kernel.org/linux-riscv/12784326.9UPPK3MAeB@basile.remlab.net/ # [1] >> > Suggested-by: Palmer Dabbelt <palmer@rivosinc.com> >> > Suggested-by: Rémi Denis-Courmont <remi@remlab.net> >> > Signed-off-by: Björn Töpel <bjorn@rivosinc.com> >> > --- >> > >> > I figured I'd sent out a proper patch. I like Andy's optimization >> > patch, but TBH I think we should do that as a follow up. >> > >> > As Rémi pointed out, the clearing might be opted out, but I left it in >> > here. >> >> I think we're going to end up with a bunch of uarch-specific stuff here, but >> for now having the heavy hammer seems safest. There's no V hardware yet so >> we can't really tell how anything performs, at least this way we're >> definately not leaking anything. >> >> So I'm OK with this. I'd also be fine with clearing to all-1s, I think it's >> kind of splitting hairs at this point: the 1s are nice because they're what >> the rest of V does, but setting vill should make everything trap anyway so >> maybe it doesn't matter -- and it's not clear if 1 or 0 will allow initial, >> so who knows. >> >> Darius: I'm cool swapping over to the 1s if you feel strongly about it. >> Bjorn says Sweeden is on vacation, so just LMK and I'll re-spin it with the >> 1s. > > I think all 1s would be preferred, but I don't think it's particularly > critical (splitting hairs, like you said), so I'll let you make the call. OK, I'm just going to take this then as it's the less work option ;) > >> >> Regardless I'd like to pick up something that blows away V state for this >> merge window, as it'll make sure the uABI is quite strictly enforced. >> >> > Björn >> > >> > --- >> > arch/riscv/include/asm/vector.h | 25 +++++++++++++++++++++++++ >> > arch/riscv/kernel/traps.c | 2 ++ >> > 2 files changed, 27 insertions(+) >> > >> > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h >> > index 04c0b07bf6cd..692ce55e4a69 100644 >> > --- a/arch/riscv/include/asm/vector.h >> > +++ b/arch/riscv/include/asm/vector.h >> > @@ -163,6 +163,30 @@ static inline void __switch_to_vector(struct task_struct *prev, >> > void riscv_v_vstate_ctrl_init(struct task_struct *tsk); >> > bool riscv_v_vstate_ctrl_user_allowed(void); >> > >> > +static inline void riscv_v_vstate_discard(struct pt_regs *regs) >> > +{ >> > + unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1); >> > + >> > + if (!riscv_v_vstate_query(regs)) >> > + return; >> > + >> > + riscv_v_enable(); >> > + asm volatile ( >> > + ".option push\n\t" >> > + ".option arch, +v\n\t" >> > + "vsetvli %0, x0, e8, m8, ta, ma\n\t" >> > + "vmv.v.i v0, 0\n\t" >> > + "vmv.v.i v8, 0\n\t" >> > + "vmv.v.i v16, 0\n\t" >> > + "vmv.v.i v24, 0\n\t" >> > + "vsetvl %0, x0, %1\n\t" >> > + ".option pop\n\t" >> > + : "=&r" (vl) : "r" (vtype_inval) : "memory"); >> > + riscv_v_disable(); >> > + >> > + riscv_v_vstate_on(regs); >> > +} >> > + >> > #else /* ! CONFIG_RISCV_ISA_V */ >> > >> > struct pt_regs; >> > @@ -178,6 +202,7 @@ static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; } >> > #define __switch_to_vector(__prev, __next) do {} while (0) >> > #define riscv_v_vstate_off(regs) do {} while (0) >> > #define riscv_v_vstate_on(regs) do {} while (0) >> > +#define riscv_v_vstate_discard(regs) do {} while (0) >> > >> > #endif /* CONFIG_RISCV_ISA_V */ >> > >> > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c >> > index 05ffdcd1424e..00c68b57ff88 100644 >> > --- a/arch/riscv/kernel/traps.c >> > +++ b/arch/riscv/kernel/traps.c >> > @@ -295,6 +295,8 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs) >> > regs->epc += 4; >> > regs->orig_a0 = regs->a0; >> > >> > + riscv_v_vstate_discard(regs); >> > + >> > syscall = syscall_enter_from_user_mode(regs, syscall); >> > >> > if (syscall < NR_syscalls) >> > >> > base-commit: 4681dacadeefa5ca6017e00736adc1d7dc963c6a > > -- > You received this message because you are subscribed to the Google Groups "linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux+unsubscribe@rivosinc.com. > To view this discussion on the web visit https://groups.google.com/a/rivosinc.com/d/msgid/linux/ZJSLKrB/4xaYB75d%40bruce.bluespec.com. > For more options, visit https://groups.google.com/a/rivosinc.com/d/optout.
On 6/22/23 10:36, Björn Töpel wrote: > From: Björn Töpel <bjorn@rivosinc.com> > > The RISC-V vector specification states: > Executing a system call causes all caller-saved vector registers > (v0-v31, vl, vtype) and vstart to become unspecified. So the only use case of saving/restoring V regs in __switch_to is an interrupt induced task switch ? > > The vector registers are cleared, vill is set (invalid), and the > vector status is set to Initial. > > That way we can prevent userspace from accidentally relying on the > stated save. > > Rémi pointed out [1] that clearing the registers might be superfluous, > and setting vill is sufficient. > > Link: https://lore.kernel.org/linux-riscv/12784326.9UPPK3MAeB@basile.remlab.net/ # [1] > Suggested-by: Palmer Dabbelt <palmer@rivosinc.com> > Suggested-by: Rémi Denis-Courmont <remi@remlab.net> > Signed-off-by: Björn Töpel <bjorn@rivosinc.com> > --- > > I figured I'd sent out a proper patch. I like Andy's optimization > patch, but TBH I think we should do that as a follow up. > > As Rémi pointed out, the clearing might be opted out, but I left it in > here. > > > Björn > > --- > arch/riscv/include/asm/vector.h | 25 +++++++++++++++++++++++++ > arch/riscv/kernel/traps.c | 2 ++ > 2 files changed, 27 insertions(+) > > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h > index 04c0b07bf6cd..692ce55e4a69 100644 > --- a/arch/riscv/include/asm/vector.h > +++ b/arch/riscv/include/asm/vector.h > @@ -163,6 +163,30 @@ static inline void __switch_to_vector(struct task_struct *prev, > void riscv_v_vstate_ctrl_init(struct task_struct *tsk); > bool riscv_v_vstate_ctrl_user_allowed(void); > > +static inline void riscv_v_vstate_discard(struct pt_regs *regs) > +{ > + unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1); > + > + if (!riscv_v_vstate_query(regs)) > + return; > + > + riscv_v_enable(); > + asm volatile ( > + ".option push\n\t" > + ".option arch, +v\n\t" > + "vsetvli %0, x0, e8, m8, ta, ma\n\t" > + "vmv.v.i v0, 0\n\t" > + "vmv.v.i v8, 0\n\t" > + "vmv.v.i v16, 0\n\t" > + "vmv.v.i v24, 0\n\t" > + "vsetvl %0, x0, %1\n\t" > + ".option pop\n\t" > + : "=&r" (vl) : "r" (vtype_inval) : "memory"); > + riscv_v_disable(); > + > + riscv_v_vstate_on(regs); > +} > + > #else /* ! CONFIG_RISCV_ISA_V */ > > struct pt_regs; > @@ -178,6 +202,7 @@ static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; } > #define __switch_to_vector(__prev, __next) do {} while (0) > #define riscv_v_vstate_off(regs) do {} while (0) > #define riscv_v_vstate_on(regs) do {} while (0) > +#define riscv_v_vstate_discard(regs) do {} while (0) > > #endif /* CONFIG_RISCV_ISA_V */ > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 05ffdcd1424e..00c68b57ff88 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -295,6 +295,8 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs) > regs->epc += 4; > regs->orig_a0 = regs->a0; > > + riscv_v_vstate_discard(regs); > + > syscall = syscall_enter_from_user_mode(regs, syscall); > > if (syscall < NR_syscalls) > > base-commit: 4681dacadeefa5ca6017e00736adc1d7dc963c6a
Vineet Gupta <vineetg@rivosinc.com> writes: > On 6/22/23 10:36, Björn Töpel wrote: >> From: Björn Töpel <bjorn@rivosinc.com> >> >> The RISC-V vector specification states: >> Executing a system call causes all caller-saved vector registers >> (v0-v31, vl, vtype) and vstart to become unspecified. > > So the only use case of saving/restoring V regs in __switch_to is an > interrupt induced task switch ? Yes!
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h index 04c0b07bf6cd..692ce55e4a69 100644 --- a/arch/riscv/include/asm/vector.h +++ b/arch/riscv/include/asm/vector.h @@ -163,6 +163,30 @@ static inline void __switch_to_vector(struct task_struct *prev, void riscv_v_vstate_ctrl_init(struct task_struct *tsk); bool riscv_v_vstate_ctrl_user_allowed(void); +static inline void riscv_v_vstate_discard(struct pt_regs *regs) +{ + unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1); + + if (!riscv_v_vstate_query(regs)) + return; + + riscv_v_enable(); + asm volatile ( + ".option push\n\t" + ".option arch, +v\n\t" + "vsetvli %0, x0, e8, m8, ta, ma\n\t" + "vmv.v.i v0, 0\n\t" + "vmv.v.i v8, 0\n\t" + "vmv.v.i v16, 0\n\t" + "vmv.v.i v24, 0\n\t" + "vsetvl %0, x0, %1\n\t" + ".option pop\n\t" + : "=&r" (vl) : "r" (vtype_inval) : "memory"); + riscv_v_disable(); + + riscv_v_vstate_on(regs); +} + #else /* ! CONFIG_RISCV_ISA_V */ struct pt_regs; @@ -178,6 +202,7 @@ static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; } #define __switch_to_vector(__prev, __next) do {} while (0) #define riscv_v_vstate_off(regs) do {} while (0) #define riscv_v_vstate_on(regs) do {} while (0) +#define riscv_v_vstate_discard(regs) do {} while (0) #endif /* CONFIG_RISCV_ISA_V */ diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 05ffdcd1424e..00c68b57ff88 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -295,6 +295,8 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs) regs->epc += 4; regs->orig_a0 = regs->a0; + riscv_v_vstate_discard(regs); + syscall = syscall_enter_from_user_mode(regs, syscall); if (syscall < NR_syscalls)