Re: [PATCH v6 08/13] xen/arm: Fold mmu_init_secondary_cpu() to head.S

[Julien Grall <julien@xxxxxxx>]

On 31/08/2023 10:16, Henry Wang wrote:
> > On Aug 31, 2023, at 17:12, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:
> >
> > Hi Henry,
> >
> > On 28/08/2023 02:32, Henry Wang wrote:
> > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > > index 33b038e7e0..39218cf15f 100644
> > > --- a/xen/arch/arm/arm32/head.S
> > > +++ b/xen/arch/arm/arm32/head.S
> > > @@ -83,6 +83,25 @@
> > >           isb
> > >   .endm
> > >
> > > +/*
> > > + * Enforce Xen page-tables do not contain mapping that are both
> > > + * Writable and eXecutables.
> > > + *
> > > + * This should be called on each secondary CPU.
> > > + */
> > > +.macro pt_enforce_wxn tmp
> > > +        mrc   CP32(\tmp, HSCTLR)
> > > +        orr   \tmp, \tmp, #SCTLR_Axx_ELx_WXN
> > > +        dsb
> > > +        mcr   CP32(\tmp, HSCTLR)
> > > +        /*
> > > +         * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized
> > > +         * before flushing the TLBs.
> > > +         */
> > > +        isb
> > > +        flush_xen_tlb_local \tmp
> > > +.endm
> > > +
> > >   /*
> > >    * Common register usage in this file:
> > >    *   r0  -
> > > @@ -254,6 +273,7 @@ secondary_switched:
> > >           /* Use a virtual address to access the UART. */
> > >           mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
> > >   #endif
> > > +        pt_enforce_wxn
> > >   
> > Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the MMU related 
> > functionality are bundled together?
> >
> > Also AFAIU, mov_w has not effect on pt_enforce_wxn().
> >
> > So that I can create a function "enable_secondary_cpu_mm()" - similar to one 
> > you introduced for arm64
> Sure, I am good with this if other maintainers do not have any objections.
I am objecting. It would be quite handy to print a message on the 
console to indicate that we are enforce WXN. So we want to update UART 
address (stored in r11) before hand.
Cheers,

--
Julien Grall