[PATCH -next v20 11/26] riscv: Allocate user's vector context in the first-use trap

[Andy Chiu]

Hi Conor,

On Fri, May 19, 2023 at 1:47 AM Conor Dooley <conor at kernel.org> wrote:
>
> On Thu, May 18, 2023 at 04:19:34PM +0000, Andy Chiu wrote:
> > Vector unit is disabled by default for all user processes. Thus, a
> > process will take a trap (illegal instruction) into kernel at the first
> > time when it uses Vector. Only after then, the kernel allocates V
> > context and starts take care of the context for that user process.
> >
> > Suggested-by: Richard Henderson <richard.henderson at linaro.org>
> > Link: https://lore.kernel.org/r/3923eeee-e4dc-0911-40bf-84c34aee962d@linaro.org
> > Signed-off-by: Andy Chiu <andy.chiu at sifive.com>
> > ---
> > Hey Heiko and Conor, I am dropping you guys' A-b, T-b, and R-b because I
> > added a check in riscv_v_first_use_handler().
>
> > +bool riscv_v_first_use_handler(struct pt_regs *regs)
> > +{
> > +     u32 __user *epc = (u32 __user *)regs->epc;
> > +     u32 insn = (u32)regs->badaddr;
> > +
> > +     /* Do not handle if V is not supported, or disabled */
> > +     if (!has_vector() || !(elf_hwcap & COMPAT_HWCAP_ISA_V))
> > +             return false;
>
> Remind me please, in what situation is this actually even possible?
> The COMPAT_HWCAP_ISA_V flag only gets set if CONFIG_RISCV_ISA_V is
> enabled & v is in the DT.
> has_vector() is backed by different things whether alternatives are
> enabled or not. With alternatives, it depends on the bit being set in
> the riscv_isa bitmap & the Kconfig option.
> Without alternatives it is backed by __riscv_isa_extension_available()
> which only depends in the riscv_isa bitmap.
> Since the bit in the bitmap does not get cleared if CONFIG_RISCV_ISA_V
> is not set, unlike the elf_hwcap bit which does, it seems like this
> might be the condition you are trying to prevent?
>

In fact the case you mentioned is prevented by Kconfig itself. To be
more specific, riscv_v_first_use_handler() always returns false if
CONFIG_RISCV_ISA_V is not set. In such config, the function is defined
as an inline that returns false in include/asm/vector.h, and
kernl/vector.c is not compiled.

The case that I intended to protect is another scenario. e.g. If a
multicore system has different VLENs across cores, with
CONFIG_RISCV_ISA_V set. Since this series assumes an SMP system, it
turns off V in ELF_HWCAP if it detects uneven VLENs during smp boot.
In this case we must not handle the first-use trap if the user still
executes V instruction anyway.

> If so,
> Reviewed-by: Conor Dooley <conor.dooley at microchip.com>
>
> Otherwise, please let me know where I have gone wrong!
>
> Thanks,
> Conor.

Thanks,
Andy