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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2023-07-26 at 13:58 +0200, Jan Beulich wrote:
> On 26.07.2023 13:23, Oleksii wrote:
> > I would like to ask for advice on whether it would be easier, less
> > bug-
> > provoking ( during identity mapping to remove of whole Xen ) to
> > have a
> > separate identity section that won't be more than PAGE_SIZE.
> 
> I'm afraid you can't safely do this in C, or at least not without
> further checking on what the compiler actually did.
> 
> > @@ -264,6 +268,19 @@ void __init enable_mmu(void)
> >                RV_STAGE1_MODE << SATP_MODE_SHIFT);
> >  }
> >  
> > +void __attribute__((naked)) __section(".ident")
> > turn_on_mmu(unsigned
> > long ra)
> 
> Did you read what gcc doc says about "naked"? Extended asm() isn't
> supported there. Since ...
> 
> > +{
> > +    /* Ensure page table writes precede loading the SATP */
> > +    sfence_vma();
> > +
> > +    /* Enable the MMU and load the new pagetable for Xen */
> > +    csr_write(CSR_SATP,
> > +              PFN_DOWN((unsigned long)stage1_pgtbl_root) |
> > +              RV_STAGE1_MODE << SATP_MODE_SHIFT);
> > +
> > +    asm volatile( "jr %0\n" : : "r"(ra) );
> > +}
> 
> ... none of this really requires C, I think we're at the point where
> (iirc) Andrew's and my suggestion wants following, moving this to
> assembly code (at which point it doesn't need to be a separate
> function). You can still build page tables in C, of course. (Likely
> you then also won't need a separate section; some minimal alignment
> guarantees ought to suffice to make sure the critical code is
> confined to a single page.)

Thanks. I'll move all of this to assembly code.
Regarding alignment it is needed alignment on start and end of
function:
    .balign PAGE_SIZE
    GLOBAL(turn_on_mmu)
        ...
    .balign PAGE_SIZE
    ENDPROC(turn_on_mmu)

Does the better way exist?

~ Oleksii