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

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Julien,

> On Sep 11, 2023, at 22:51, Julien Grall <julien@xxxxxxx> 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.

Good idea about the printing, I am happy to add a printed message on top of the 
macro saying that we are enforcing WXN from here if you agree.

Kind regards,
Henry

> 
> Cheers,
> 
> -- 
> Julien Grall