lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHCEehLnX6hAr6uRYy4BxzuBhTp64wn0yCVGJ85+0aS88OeKtA@mail.gmail.com>
Date:   Tue, 16 Nov 2021 21:14:29 +0800
From:   Greentime Hu <greentime.hu@...ive.com>
To:     Ben Dooks <ben.dooks@...ethink.co.uk>
Cc:     Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Albert Ou <aou@...s.berkeley.edu>
Subject: Re: [PATCH v9 16/17] riscv: Fix an illegal instruction exception when
 accessing vlenb without enable vector first

Ben Dooks <ben.dooks@...ethink.co.uk> 於 2021年11月10日 週三 下午6:38寫道:
>
> On 09/11/2021 09:48, Greentime Hu wrote:
> > It triggered an illegal instruction exception when accessing vlenb CSR
> > without enable vector first. To fix this issue, we should enable vector
> > before using it and disable vector after using it.
> >
> > Co-developed-by: Vincent Chen <vincent.chen@...ive.com>
> > Signed-off-by: Vincent Chen <vincent.chen@...ive.com>
> > Signed-off-by: Greentime Hu <greentime.hu@...ive.com>
> > ---
> >   arch/riscv/include/asm/vector.h        | 2 ++
> >   arch/riscv/kernel/cpufeature.c         | 2 ++
> >   arch/riscv/kernel/kernel_mode_vector.c | 6 ++++--
> >   3 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> > index 5d7f14453f68..ca063c8f47f2 100644
> > --- a/arch/riscv/include/asm/vector.h
> > +++ b/arch/riscv/include/asm/vector.h
> > @@ -8,6 +8,8 @@
> >
> >   #include <linux/types.h>
> >
> > +void rvv_enable(void);
> > +void rvv_disable(void);
> >   void kernel_rvv_begin(void);
> >   void kernel_rvv_end(void);
> >
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 8e7557980faf..0139ec20adce 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -159,7 +159,9 @@ void __init riscv_fill_hwcap(void)
> >       if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
> >               static_branch_enable(&cpu_hwcap_vector);
> >               /* There are 32 vector registers with vlenb length. */
> > +             rvv_enable();
> >               riscv_vsize = csr_read(CSR_VLENB) * 32;
> > +             rvv_disable();
> >       }
> >   #endif
>
> Would it be better to enable this here, and then restore the original
> state instead of calling rvv_disable? If it was enabled then maybe
> something else is going to rely on that state?
>
> Maybe something like:
>
>                 prev = rvv_enable()
>                 riscv_vsize = csr_read(CSR_VLENB) * 32;
>                 rvv_restore(prev);
>
>
Hi Ben,

Thank you for reviewing this. The rvv won't be enabled here because we
disable it in head.S at beginning and this stage is still doing some
initialization work for rvv to set riscv_vsize, so we can assume that
kernel mode vector should not be allowed to use vector yet.

> >   }
> > diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> > index 8d2e53ea25c1..1ecb6ec5c56d 100644
> > --- a/arch/riscv/kernel/kernel_mode_vector.c
> > +++ b/arch/riscv/kernel/kernel_mode_vector.c
> > @@ -71,15 +71,17 @@ static void put_cpu_vector_context(void)
> >       preempt_enable();
> >   }
> >
> > -static void rvv_enable(void)
> > +void rvv_enable(void)
> >   {
> >       csr_set(CSR_STATUS, SR_VS);
> >   }
> > +EXPORT_SYMBOL(rvv_enable);
> >
> > -static void rvv_disable(void)
> > +void rvv_disable(void)
> >   {
> >       csr_clear(CSR_STATUS, SR_VS);
> >   }
> > +EXPORT_SYMBOL(rvv_disable);
> >
> >   /*
> >    * kernel_rvv_begin(): obtain the CPU vector registers for use by the calling
> >
>
>
> --
> Ben Dooks                               http://www.codethink.co.uk/
> Senior Engineer                         Codethink - Providing Genius
>
> https://www.codethink.co.uk/privacy.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ