From: Palmer Dabbelt <palmer@dabbelt.com>
To: Vineet Gupta <vineetg@rivosinc.com>
Cc: stillson@rivosinc.com, Paul Walmsley <paul.walmsley@sifive.com>,
anup@brainfault.org, atishp@atishpatra.org, guoren@kernel.org,
Conor Dooley <conor.dooley@microchip.com>,
greentime.hu@sifive.com, vincent.chen@sifive.com,
andy.chiu@sifive.com, Andrew Waterman <andrew@sifive.com>,
Darius Rad <darius@bluespec.com>,
arnd@kernel.org, linux-kernel@vger.kernel.org,
linux-riscv@lists.infradead.org, bjorn@kernel.org,
fweimer@redhat.com, libc-alpha@sourceware.org,
Conor Dooley <conor.dooley@microchip.com>,
christoph.muellner@vrull.eu, Aaron Durbin <adurbin@rivosinc.com>,
linux@rivosinc.com
Subject: Re: RISCV Vector unit disabled by default for new task (was Re: [PATCH v12 17/17] riscv: prctl to enable vector commands)
Date: Thu, 08 Dec 2022 22:27:26 -0800 (PST) [thread overview]
Message-ID: <mhng-84ad9495-ef4b-4343-89ee-dfe45ab69ff7@palmer-ri-x1c9> (raw)
In-Reply-To: <b1dae947-d52a-d28e-5ddc-c1ad6d29828c@rivosinc.com>
On Thu, 08 Dec 2022 21:16:06 PST (-0800), Vineet Gupta wrote:
> Hi Darius, Andrew, Palmer
>
> On 9/21/22 14:43, Chris Stillson wrote:
>> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
>>
>> @@ -134,7 +135,6 @@ void start_thread(struct pt_regs *regs, unsigned long pc,
>> if (WARN_ON(!vstate->datap))
>> return;
>> }
>> - regs->status |= SR_VS_INITIAL;
>>
>
> Perhaps not obvious from the patch, but this is a major user experience
> change: As in V unit would be turned off for a new task and we will rely
> on a userspace prctl (also introduced in this patch) to enable V.
IMO that's the only viable option: enabling V adds more user-visible
state, which is a uABI break. I haven't really had time to poke through
all the versions here, but I'd have the call look something like
prctl(RISCV_ENABLE_V, min_vlenb, max_vlenb, flags);
where
* min_vlenb is the smallest VLENB that userspace can support. There's
alreday an LLVM argument for this, I haven't dug into the generated
code but I assume it'll blow up on smaller VLENB systems somehow.
* max_vlenb is the largest VLENB that userspace can support.
* flags is just a placeholder for now, with 0 meaning "V as defined by
1.0 for all threads in this proces". That should give us an out if
something more complicated happens in the future.
That way VLA code can call `prctl(RISCV_ENABLE_V, 128, 8192, 0)` as it
supports any V 1.0 implementation, while code with other constraints can
avoid having V turned on in an unsupported configuration.
I think we can start out with no flags, but there's a few I could see
being useful already:
* Cross process/thread enabling. I think a reasonable default is
"enable V for all current and future threads in this process", but one
could imagine flags for "just this thread" vs "all current threads", a
default for new threads, and a default for child processes. I don't
think it matters so much what we pick as a default, just that it's
written down.
* Setting the VLENB bounds vs updating them. I'm thinking for shared
libraries, where they'd only want to enable V in the shared library if
it's already in a supported configuration. I'm not sure what the
right rules are here, but again it's best to write that down.
* Some way to disable V. Maybe we just say `prctl(RISCV_ENABLE_V, 0, 0,
...)` disables V, or maybe it's a flag? Again, it should just be
written down.
* What exactly we're enabling -- is it the V extension, or just the V
registers?
There's a bunch of subtly here, though, so I think we'd at least want
glibc and gdb support posted before committing to any uABI. It's
probably also worth looking at what the Arm folks did for SVE: I gave it
a quick glance and it seems like there's a lot of similarities with what
I'm suggesting here, but again a lot of this is pretty subtle stuff so
it's hard to tell just at a glance.
> I know some of you had different opinion on this in the past [1], so
> this is to make sure everyone's on same page.
> And if we agree this is the way to go, how exactly will this be done in
> userspace.
>
> glibc dynamic loader will invoke the prctl() ? How will it decide
> whether to do this (or not) - will it be unconditional or will it use
> the hwcap - does latter plumbing exist already ? If so is it AT_HWCAP /
> HWCAP2.
That part I haven't sorted out yet, and I don't think it's sufficient to
just say "userspace should enable what it can support" because of how
pervasive V instructions are going to be.
I don't think we need HWCAP, as userspace will need to call the prctl()
anyway to turn on V and thus can just use the success/failure of that to
sort things out.
Maybe it's sufficient to rely on some sort of sticky prctl() (or sysctl
type thing, the differences there would be pretty subtle) and just not
worry about it, but having some way of encoding this in the ELF seems
nice. That said, we've had a bunch of trouble sorting out the ISA
encoding in ELFs so maybe it's just not worth bothering?
> Also for static linked executables, where will the prctl be called from ?
I guess that's pretty far in the weeds, but we could at least hook CRT
to insert the relevant code. We'd really need to sort out how we're
going to encode the V support in binaries, though.
> [1] https://sourceware.org/pipermail/libc-alpha/2021-November/132883.html
next prev parent reply other threads:[~2022-12-09 6:27 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20220921214439.1491510-1-stillson@rivosinc.com>
[not found] ` <20220921214439.1491510-17-stillson@rivosinc.com>
2022-12-09 5:16 ` Vineet Gupta
2022-12-09 6:27 ` Palmer Dabbelt [this message]
2022-12-09 7:42 ` Andrew Waterman
2022-12-09 10:02 ` Florian Weimer
2022-12-09 12:21 ` Darius Rad
2022-12-09 12:32 ` Florian Weimer
2022-12-09 12:42 ` Darius Rad
2022-12-09 13:04 ` Florian Weimer
2022-12-09 17:21 ` Palmer Dabbelt
2022-12-09 19:42 ` Vineet Gupta
2022-12-09 19:58 ` Andrew Waterman
2022-12-13 16:43 ` Darius Rad
2022-12-14 20:07 ` Vineet Gupta
2022-12-14 23:13 ` Samuel Holland
2022-12-15 2:09 ` Darius Rad
2022-12-15 11:48 ` Björn Töpel
2022-12-15 12:28 ` Florian Weimer
2022-12-15 15:33 ` Richard Henderson
2022-12-15 18:57 ` Vineet Gupta
2022-12-15 18:59 ` Andrew Pinski
2022-12-15 19:01 ` Andrew Pinski
2022-12-15 19:56 ` Richard Henderson
2022-12-09 13:58 ` Icenowy Zheng
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=mhng-84ad9495-ef4b-4343-89ee-dfe45ab69ff7@palmer-ri-x1c9 \
--to=palmer@dabbelt.com \
--cc=adurbin@rivosinc.com \
--cc=andrew@sifive.com \
--cc=andy.chiu@sifive.com \
--cc=anup@brainfault.org \
--cc=arnd@kernel.org \
--cc=atishp@atishpatra.org \
--cc=bjorn@kernel.org \
--cc=christoph.muellner@vrull.eu \
--cc=conor.dooley@microchip.com \
--cc=darius@bluespec.com \
--cc=fweimer@redhat.com \
--cc=greentime.hu@sifive.com \
--cc=guoren@kernel.org \
--cc=libc-alpha@sourceware.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux@rivosinc.com \
--cc=paul.walmsley@sifive.com \
--cc=stillson@rivosinc.com \
--cc=vincent.chen@sifive.com \
--cc=vineetg@rivosinc.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 read-only IMAP folder(s) and NNTP newsgroup(s).