Re: [PATCH v2 1/2] xen/arm32: head: Introduce enable_{boot,secondary}_cpu_mm()

[Julien Grall <julien@xxxxxxx>]

Hi,

On 06/11/2023 06:46, Michal Orzel wrote:
> On 03/11/2023 18:34, Ayan Kumar Halder wrote:
> > +/*
> > + * Enable mm (turn on the data cache and the MMU) for the boot CPU.
> > + * The function will return to the virtual address provided in LR (e.g. the
> > + * runtime mapping).
> > + *
> > + * Inputs:
> > + *   lr : Virtual address to return to.
> > + *
> > + * Clobbers r0 - r6
> > + */
> > +enable_boot_cpu_mm:
> > +        mov   r6, lr
> > +
> > +        bl    create_page_tables
> > +
> > +        /* Address in the runtime mapping to jump to after the MMU is enabled 
> > */
> > +        mov_w lr, 1f
> > +        b     enable_mmu
> > +1:
> > +        /* Return to the virtual address requested by the caller. */
> I find this comment a bit misleading as it reads as if this instruction was 
> causing a return.
For the arm64 side, we have the comment on top of the branch 
instruction. I would suggest the following:
"
Prepare the fixmap. The function will return to the virtual address 
requested by the caller.
"

> Apart from that, this change LGTM. Depending on the order of other arm32 head.S 
> patches:
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
>
> ~Michal
--
Julien Grall