Re: [PATCH v5 2/2] xen/riscv: introduce identity mapping

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-07-31 at 16:07 +0200, Jan Beulich wrote:
> On 27.07.2023 15:38, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/riscv64/head.S
> > +++ b/xen/arch/riscv/riscv64/head.S
> > @@ -39,6 +39,23 @@ ENTRY(start)
> >          jal     calc_phys_offset
> >          mv      s2, a0
> >  
> > +        jal     setup_initial_pagetables
> > +
> > +        /* Calculate proper VA after jump from 1:1 mapping */
> > +        la      t0, .L_primary_switched
> > +        sub     t0, t0, s2
> > +
> > +        mv      a0, t0
> > +        jal     turn_on_mmu
> 
> Any reason you don't do the calculation right in a0?
Probably it was before. But you are right there is no any sense in
using of t0 in the current code.
I'll update that. Thanks.

> 
> > @@ -54,3 +71,18 @@ ENTRY(reset_stack)
> >  
> >          ret
> >  
> > +        .section .text.ident, "ax", %progbits
> > +
> > +ENTRY(turn_on_mmu)
> > +        sfence.vma
> > +
> > +        li      t0, RV_STAGE1_MODE
> > +        li      t1, SATP_MODE_SHIFT
> > +        sll     t0, t0, t1
> 
> Can't the last two be folded to
> 
>         slli     t0, t0, SATP_MODE_SHIFT
> 
> (I don't recall what li's valid value range is, so I'm not sure if
> 
>         li      t0, RV_STAGE1_MODE << SATP_MODE_SHIFT
> 
> would be an option.)
Both of options will work but I prefer to use SLLI as LI expands into a
potentially long and inefficient shift-and-add sequence ( but in this
case I think this is not so important ).
> 
> Everything else looks good to me now, but will of course want a
> maintainer looking over.
Would it be better to send a new version now or wait for a response
from other maintainers?

~ Oleksii