linux-kernel - Re: [PATCH] RISC-V: Add the directive for alignment of stvec's value

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]

Date:   Thu, 2 Aug 2018 09:26:48 +0800
From:   Zong Li <zongbox@...il.com>
To:     Palmer Dabbelt <palmer@...ive.com>
Cc:     Zong Li <zong@...estech.com>, linux-riscv@...ts.infradead.org,
        aou@...s.berkeley.edu,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        greentime@...estech.com
Subject: Re: [PATCH] RISC-V: Add the directive for alignment of stvec's value

Palmer Dabbelt <palmer@...ive.com> 於 2018年8月2日 週四 上午8:38寫道：
>
> On Wed, 20 Jun 2018 18:40:07 PDT (-0700), zong@...estech.com wrote:
> > The stvec's value must be 4 byte alignment by specification definition.
> > This directive avoids to stvec be set the non-alignment value by the
> > following code in head.S
> >
> >  /* Point stvec to virtual address of intruction after satp write */
> > la a0, 1f
> > add a0, a0, a1
> > csrw stvec, a0
> >
> > Signed-off-by: Zong Li <zong@...estech.com>
> > ---
> >  arch/riscv/kernel/head.S | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index 396ec7b349ce..ae7b204f531c 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -94,6 +94,7 @@ relocate:
> >       or a0, a0, a1
> >       sfence.vma
> >       csrw sptbr, a0
> > +.align 2
> >  1:
> >       /* Set trap vector to spin forever to help debug */
> >       la a0, .Lsecondary_park
>
> I don't think this is actually correct: you shouldn't be aligning the address
> at which stvec is set, but the address that stvec is set to.  If this is fixing
> anything then it's probably just by coincidence as it's causing
> .Lsecondary_park to be aligned.
>
> I think this patch is the correct fix
>
>     diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>     index 6e07ed37bbff..d1beecf1d060 100644
>     --- a/arch/riscv/kernel/head.S
>     +++ b/arch/riscv/kernel/head.S
>     @@ -143,6 +143,7 @@ relocate:
>         tail smp_callin
>      #endif
>
>     +.align 2
>      .Lsecondary_park:
>         /* We lack SMP support or have too many harts, so park this hart */
>         wfi
>
> Thanks for pointing this out!
>
The position which I set in this patch is also be set to stvec for jumping to
correct VA at satp enabling. The label .Lsecondary_park is also be set to stvec,
I don't notice it because as you said, this label address is correct
just by coincidence.
I think there are two places need to be aligned.

The first setting as following:
relocate:
        ...
        /* Point stvec to virtual address of intruction after satp write */
        la a0, 1f
        add a0, a0, a1
        csrw stvec, a0    <-----------  first set
        ...
        sfence.vma
        csrw sptbr, a0
1:
        /* Set trap vector to spin forever to help debug */
        la a0, .Lsecondary_park
        csrw stvec, a0