All the EL2 MMU specific registers in head.S are enclosed within CONFIG_MMU.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
 xen/arch/arm/arm32/head.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 4ff5c220bc..1d0f84b18f 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -224,6 +224,7 @@ cpu_init_done:
         mcr   CP32(r0, HMAIR0)
         mcr   CP32(r1, HMAIR1)
 
+#ifdef CONFIG_MMU
         /*
          * Set up the HTCR:
          * PT walks use Inner-Shareable accesses,
@@ -232,6 +233,7 @@ cpu_init_done:
          */
         mov_w r0, (TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
         mcr   CP32(r0, HTCR)
+#endif
 
         mov_w r0, HSCTLR_SET
         mcr   CP32(r0, HSCTLR)
-- 
2.25.1

Hi Ayan,

> On 4 Feb 2025, at 19:23, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
> 
> All the EL2 MMU specific registers in head.S are enclosed within CONFIG_MMU.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> xen/arch/arm/arm32/head.S | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 4ff5c220bc..1d0f84b18f 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -224,6 +224,7 @@ cpu_init_done:
>         mcr   CP32(r0, HMAIR0)
>         mcr   CP32(r1, HMAIR1)
> 
> +#ifdef CONFIG_MMU
>         /*
>          * Set up the HTCR:
>          * PT walks use Inner-Shareable accesses,
> @@ -232,6 +233,7 @@ cpu_init_done:
>          */
>         mov_w r0, (TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
>         mcr   CP32(r0, HTCR)
> +#endif

I was wondering if here it was better, for readability, to have this part defined in the arm32/mmu/head.S and
arm32/mpu/head.S could have implemented a stub, maybe the maintainer could help with that.

Anyway this solution works for me.

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

> 
>         mov_w r0, HSCTLR_SET
>         mcr   CP32(r0, HSCTLR)
> -- 
> 2.25.1
> 
> 

Hi,

On 06/02/2025 14:48, Luca Fancellu wrote:
>> On 4 Feb 2025, at 19:23, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>
>> All the EL2 MMU specific registers in head.S are enclosed within CONFIG_MMU.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>> xen/arch/arm/arm32/head.S | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>> index 4ff5c220bc..1d0f84b18f 100644
>> --- a/xen/arch/arm/arm32/head.S
>> +++ b/xen/arch/arm/arm32/head.S
>> @@ -224,6 +224,7 @@ cpu_init_done:
>>          mcr   CP32(r0, HMAIR0)
>>          mcr   CP32(r1, HMAIR1)
>>
>> +#ifdef CONFIG_MMU
>>          /*
>>           * Set up the HTCR:
>>           * PT walks use Inner-Shareable accesses,
>> @@ -232,6 +233,7 @@ cpu_init_done:
>>           */
>>          mov_w r0, (TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
>>          mcr   CP32(r0, HTCR)
>> +#endif
> 
> I was wondering if here it was better, for readability, to have this part defined in the arm32/mmu/head.S and
> arm32/mpu/head.S could have implemented a stub, maybe the maintainer could help with that.

The current logic is a bit odd because the MM specific registers are 
initialized in two different places (cpu_init and enable_mmu).

It would be better if we have a single place. So I would move setting 
HTCR (and event HMAIR{0,1} even if it means duplication) to enable_mmu.

The same would apply for arm64.

Cheers,

-- 
Julien Grall