Re: [PATCH -next v19 23/24] riscv: Enable Vector code to be built

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 9, 2023 at 8:35 PM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote:
>
> Hey Andy,
>
> On Tue, May 09, 2023 at 10:30:32AM +0000, Andy Chiu wrote:
> > From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> >
> > This patch adds a config which enables vector feature from the kernel
> > space.
>
> This commit message probably needs to change, it's not exactly doing
> that anymore!

Yes, I totally missed that part. I will get commit messages updated
when it's time for the next revision.

>
> > Signed-off-by: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> > Co-developed-by: Greentime Hu <greentime.hu@xxxxxxxxxx>
> > Signed-off-by: Greentime Hu <greentime.hu@xxxxxxxxxx>
>
> > Suggested-by: Vineet Gupta <vineetg@xxxxxxxxxxxx>
> > Suggested-by: Atish Patra <atishp@xxxxxxxxxxxxxx>
>
> And I suspect that these two are also likely inaccurate at this point,
> but IDC.

Agree. I am going to drop these.

>
> > Co-developed-by: Andy Chiu <andy.chiu@xxxxxxxxxx>
> > Signed-off-by: Andy Chiu <andy.chiu@xxxxxxxxxx>
> > ---
> > Changelog V19:
> >  - Add RISCV_V_DISABLE to set compile-time default.
> >
> >  arch/riscv/Kconfig  | 31 +++++++++++++++++++++++++++++++
> >  arch/riscv/Makefile |  6 +++++-
> >  2 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 1019b519d590..fa256f2e23c1 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -466,6 +466,37 @@ config RISCV_ISA_SVPBMT
> >
> >          If you don't know what to do here, say Y.
> >
> > +config TOOLCHAIN_HAS_V
> > +     bool
> > +     default y
> > +     depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64iv)
> > +     depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32iv)
> > +     depends on LLD_VERSION >= 140000 || LD_VERSION >= 23800
> > +     depends on AS_HAS_OPTION_ARCH
> > +
> > +config RISCV_ISA_V
> > +     bool "VECTOR extension support"
> > +     depends on TOOLCHAIN_HAS_V
> > +     depends on FPU
> > +     select DYNAMIC_SIGFRAME
> > +     default y
> > +     help
> > +       Say N here if you want to disable all vector related procedure
> > +       in the kernel.
> > +
> > +       If you don't know what to do here, say Y.
> > +
> > +config RISCV_V_DISABLE
> > +     bool "Disable userspace Vector by default"
> > +     depends on RISCV_ISA_V
> > +     default n
> > +     help
> > +       Say Y here if you want to disable default enablement state of Vector
> > +       in u-mode. This way userspace has to make explicit prctl() call to
> > +       enable Vector, or enable it via sysctl interface.
>
> If we are worried about breaking userspace, why is the default for this
> option not y? Or further,
>
> config RISCV_ISA_V_DEFAULT_ENABLE
>         bool "Enable userspace Vector by default"
>         depends on RISCV_ISA_V
>         help
>           Say Y here to allow use of Vector in userspace by default.
>           Otherwise, userspace has to make an explicit prctl() call to
>           enable Vector, or enable it via the sysctl interface.
>
>           If you don't know what to do here, say N.
>

Yes, expressing the option, where Y means "on", is more direct. But I
have a little concern if we make the default as "off". Yes, we create
this option in the worries of breaking userspace. But given that the
break case might be rare, is it worth making userspace Vector harder
to use by doing this? I assume in an ideal world that nothing would
break and programs could just use V without bothering with prctl(), or
sysctl. But on the other hand, to make a program robust enough, we
must check the status with the prctl() anyway. So I have no answer
here.

> Thanks,
> Conor.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux