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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

Hi Julien,

On 11/09/2023 15:51, Julien Grall wrote:
> 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.
You mean you want to add this snippet in the current patch

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 39218cf15f..282b89a96e 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -273,6 +273,7 @@ secondary_switched:
         /* Use a virtual address to access the UART. */
         mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
 #endif
+        PRINT("- Enforce WXN -\r\n")
         pt_enforce_wxn r0
         PRINT("- Ready -\r\n")
         /* Jump to C world */

- Ayan

> Cheers,