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

[Zong Li]

Palmer Dabbelt <palmer@xxxxxxxxxx> æ 2018å8æ2æ éå äå8:38åéï
>
> On Wed, 20 Jun 2018 18:40:07 PDT (-0700), zong@xxxxxxxxxxxxx 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@xxxxxxxxxxxxx>
> > ---
> >  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