diff mbox series

[v5,09/13] xen/arm: mm: Use generic variable/function names for extendability

Message ID 20230814042536.878720-10-Henry.Wang@arm.com (mailing list archive)
State Superseded
Headers show
Series xen/arm: Split MMU code as the prepration of MPU work | expand

Commit Message

Henry Wang Aug. 14, 2023, 4:25 a.m. UTC
From: Penny Zheng <penny.zheng@arm.com>

As preparation for MPU support, which will use some variables/functions
for both MMU and MPU system, We rename the affected variable/function
to more generic names:
- init_ttbr -> init_mm,
- mmu_init_secondary_cpu() -> mm_init_secondary_cpu()
- init_secondary_pagetables() -> init_secondary_mm()
- Add a wrapper update_mm_mapping() for MMU system's
  update_identity_mapping()

Modify the related in-code comment to reflect above changes, take the
opportunity to fix the incorrect coding style of the in-code comments.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Signed-off-by: Henry Wang <Henry.Wang@arm.com>
---
v5:
- Rebase on top of xen/arm: mm: add missing extern variable declaration
v4:
- Extract the renaming part from the original patch:
  "[v3,13/52] xen/mmu: extract mmu-specific codes from mm.c/mm.h"
---
 xen/arch/arm/arm32/head.S           |  4 ++--
 xen/arch/arm/arm64/mmu/head.S       |  2 +-
 xen/arch/arm/arm64/mmu/mm.c         | 11 ++++++++---
 xen/arch/arm/arm64/smpboot.c        |  6 +++---
 xen/arch/arm/include/asm/arm64/mm.h |  7 ++++---
 xen/arch/arm/include/asm/mm.h       | 12 +++++++-----
 xen/arch/arm/mmu/mm.c               | 20 ++++++++++----------
 xen/arch/arm/smpboot.c              |  4 ++--
 8 files changed, 37 insertions(+), 29 deletions(-)

Comments

Julien Grall Aug. 21, 2023, 6:32 p.m. UTC | #1
Hi,

On 14/08/2023 05:25, Henry Wang wrote:
> From: Penny Zheng <penny.zheng@arm.com>
> 
> As preparation for MPU support, which will use some variables/functions
> for both MMU and MPU system, We rename the affected variable/function
> to more generic names:
> - init_ttbr -> init_mm,

You moved init_ttbr to mm/mmu.c. So why does this need to be renamed?

And if you really planned to use it for the MPU code. Then init_ttbr 
should not have been moved.

> - mmu_init_secondary_cpu() -> mm_init_secondary_cpu()
> - init_secondary_pagetables() -> init_secondary_mm()

The original naming were not great but the new one are a lot more 
confusing as they seem to just be a reshuffle of word.

mm_init_secondary_cpu() is only setting the WxN bit. For the MMU, I 
think it can be done much earlier. Do you have anything to add in it? If 
not, then I would consider to get rid of it.

For init_secondary_mm(), I would renamed it to prepare_secondary_mm().

> - Add a wrapper update_mm_mapping() for MMU system's
>    update_identity_mapping()
> 
> Modify the related in-code comment to reflect above changes, take the
> opportunity to fix the incorrect coding style of the in-code comments.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> ---
> v5:
> - Rebase on top of xen/arm: mm: add missing extern variable declaration
> v4:
> - Extract the renaming part from the original patch:
>    "[v3,13/52] xen/mmu: extract mmu-specific codes from mm.c/mm.h"
> ---
>   xen/arch/arm/arm32/head.S           |  4 ++--
>   xen/arch/arm/arm64/mmu/head.S       |  2 +-
>   xen/arch/arm/arm64/mmu/mm.c         | 11 ++++++++---
>   xen/arch/arm/arm64/smpboot.c        |  6 +++---
>   xen/arch/arm/include/asm/arm64/mm.h |  7 ++++---
>   xen/arch/arm/include/asm/mm.h       | 12 +++++++-----
>   xen/arch/arm/mmu/mm.c               | 20 ++++++++++----------
>   xen/arch/arm/smpboot.c              |  4 ++--
>   8 files changed, 37 insertions(+), 29 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 33b038e7e0..03ab68578a 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -238,11 +238,11 @@ GLOBAL(init_secondary)
>   secondary_switched:
>           /*
>            * Non-boot CPUs need to move on to the proper pagetables, which were
> -         * setup in init_secondary_pagetables.
> +         * setup in init_secondary_mm.
>            *
>            * XXX: This is not compliant with the Arm Arm.
>            */
> -        mov_w r4, init_ttbr          /* VA of HTTBR value stashed by CPU 0 */
> +        mov_w r4, init_mm            /* VA of HTTBR value stashed by CPU 0 */
>           ldrd  r4, r5, [r4]           /* Actual value */
>           dsb
>           mcrr  CP64(r4, r5, HTTBR)
> diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
> index ba2ddd7e67..58d91c9088 100644
> --- a/xen/arch/arm/arm64/mmu/head.S
> +++ b/xen/arch/arm/arm64/mmu/head.S
> @@ -302,7 +302,7 @@ ENDPROC(enable_mmu)
>   ENTRY(enable_secondary_cpu_mm)
>           mov   x5, lr
>   
> -        load_paddr x0, init_ttbr
> +        load_paddr x0, init_mm
>           ldr   x0, [x0]
>   
>           bl    enable_mmu
> diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c
> index 78b7c7eb00..ed0fc5ff7b 100644
> --- a/xen/arch/arm/arm64/mmu/mm.c
> +++ b/xen/arch/arm/arm64/mmu/mm.c
> @@ -106,7 +106,7 @@ void __init arch_setup_page_tables(void)
>       prepare_runtime_identity_mapping();
>   }
>   
> -void update_identity_mapping(bool enable)
> +static void update_identity_mapping(bool enable)

Why not simply renaming this function to update_mm_mapping()? But...

>   {
>       paddr_t id_addr = virt_to_maddr(_start);
>       int rc;
> @@ -120,6 +120,11 @@ void update_identity_mapping(bool enable)
>       BUG_ON(rc);
>   }
>   
> +void update_mm_mapping(bool enable)

... the new name it quite confusing. What is the mapping it is referring to?

If you don't want to keep update_identity_mapping(), then I would 
consider to simply wrap...

> +{
> +    update_identity_mapping(enable);
> +}
> +
>   extern void switch_ttbr_id(uint64_t ttbr);
>   
>   typedef void (switch_ttbr_fn)(uint64_t ttbr);
> @@ -131,7 +136,7 @@ void __init switch_ttbr(uint64_t ttbr)
>       lpae_t pte;
>   
>       /* Enable the identity mapping in the boot page tables */
> -    update_identity_mapping(true);
> +    update_mm_mapping(true);
>   
>       /* Enable the identity mapping in the runtime page tables */
>       pte = pte_of_xenaddr((vaddr_t)switch_ttbr_id);
> @@ -148,7 +153,7 @@ void __init switch_ttbr(uint64_t ttbr)
>        * Note it is not necessary to disable it in the boot page tables
>        * because they are not going to be used by this CPU anymore.
>        */
> -    update_identity_mapping(false);
> +    update_mm_mapping(false);
>   }
>   
>   /*
> diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
> index 9637f42469..2b1d086a1e 100644
> --- a/xen/arch/arm/arm64/smpboot.c
> +++ b/xen/arch/arm/arm64/smpboot.c
> @@ -111,18 +111,18 @@ int arch_cpu_up(int cpu)
>       if ( !smp_enable_ops[cpu].prepare_cpu )
>           return -ENODEV;
>   
> -    update_identity_mapping(true);
> +    update_mm_mapping(true);

... with #ifdef CONFIG_MMU here...

>   
>       rc = smp_enable_ops[cpu].prepare_cpu(cpu);
>       if ( rc )
> -        update_identity_mapping(false);
> +        update_mm_mapping(false);

... here and ...


>   
>       return rc;
>   }
>   
>   void arch_cpu_up_finish(void)
>   {
> -    update_identity_mapping(false);
> +    update_mm_mapping(false);

... here.

>   }
>   
>   /*
> diff --git a/xen/arch/arm/include/asm/arm64/mm.h b/xen/arch/arm/include/asm/arm64/mm.h
> index e0bd23a6ed..7a389c4b21 100644
> --- a/xen/arch/arm/include/asm/arm64/mm.h
> +++ b/xen/arch/arm/include/asm/arm64/mm.h
> @@ -15,13 +15,14 @@ static inline bool arch_mfns_in_directmap(unsigned long mfn, unsigned long nr)
>   void arch_setup_page_tables(void);
>   
>   /*
> - * Enable/disable the identity mapping in the live page-tables (i.e.
> - * the one pointed by TTBR_EL2).
> + * In MMU system, enable/disable the identity mapping in the live
> + * page-tables (i.e. the one pointed by TTBR_EL2) through
> + * update_identity_mapping().
>    *
>    * Note that nested call (e.g. enable=true, enable=true) is not
>    * supported.
>    */
> -void update_identity_mapping(bool enable);
> +void update_mm_mapping(bool enable);
>   
>   #endif /* __ARM_ARM64_MM_H__ */
>   
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index dc1458b047..8084c62c01 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -170,7 +170,7 @@ struct page_info
>   #define PGC_need_scrub    PGC_allocated
>   
>   /* Non-boot CPUs use this to find the correct pagetables. */
> -extern uint64_t init_ttbr;
> +extern uint64_t init_mm;
>   
>   #ifdef CONFIG_ARM_32
>   #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page))
> @@ -205,11 +205,13 @@ extern void setup_pagetables(unsigned long boot_phys_offset);
>   extern void *early_fdt_map(paddr_t fdt_paddr);
>   /* Remove early mappings */
>   extern void remove_early_mappings(void);
> -/* Allocate and initialise pagetables for a secondary CPU. Sets init_ttbr to the
> - * new page table */
> -extern int init_secondary_pagetables(int cpu);
> +/*
> + * Allocate and initialise pagetables for a secondary CPU. Sets init_mm to the
> + * new page table
> + */
> +extern int init_secondary_mm(int cpu);
>   /* Switch secondary CPUS to its own pagetables and finalise MMU setup */

Regardless what I wrote above, this comment is not accurate anymore as 
we don't switch the page tables for the secondary CPUs. We are only 
enable WxN.

In any case, this comment would need to be reworded to be more generic.

Cheers,
Henry Wang Aug. 24, 2023, 9:46 a.m. UTC | #2
Hi Julien,

> On Aug 22, 2023, at 02:32, Julien Grall <julien@xen.org> wrote:
> 
> Hi,
> 
> On 14/08/2023 05:25, Henry Wang wrote:
>> From: Penny Zheng <penny.zheng@arm.com>
>> As preparation for MPU support, which will use some variables/functions
>> for both MMU and MPU system, We rename the affected variable/function
>> to more generic names:
>> - init_ttbr -> init_mm,
> 
> You moved init_ttbr to mm/mmu.c. So why does this need to be renamed?
> 
> And if you really planned to use it for the MPU code. Then init_ttbr should not have been moved.

You are correct. I think we need to use the “init_mm” for MPU SMP support,
so I would not move this variable in v6.

> 
>> - mmu_init_secondary_cpu() -> mm_init_secondary_cpu()
>> - init_secondary_pagetables() -> init_secondary_mm()
> 
> The original naming were not great but the new one are a lot more confusing as they seem to just be a reshuffle of word.
> 
> mm_init_secondary_cpu() is only setting the WxN bit. For the MMU, I think it can be done much earlier. Do you have anything to add in it? If not, then I would consider to get rid of it.

I’ve got rid of mmu_init_secondary_cpu() function in my local v6 as it is now
folded to the assembly code.

> 
> For init_secondary_mm(), I would renamed it to prepare_secondary_mm().

Sure, thanks for the name suggestion.

> 
>>  -void update_identity_mapping(bool enable)
>> +static void update_identity_mapping(bool enable)
> 
> Why not simply renaming this function to update_mm_mapping()? But...
> 
>>  {
>>      paddr_t id_addr = virt_to_maddr(_start);
>>      int rc;
>> @@ -120,6 +120,11 @@ void update_identity_mapping(bool enable)
>>      BUG_ON(rc);
>>  }
>>  +void update_mm_mapping(bool enable)
> 
> ... the new name it quite confusing. What is the mapping it is referring to?

So I checked the MPU SMP support code and now I think I understand the reason
why update_mm_mapping() was introduced:

In the future we eventually need to support SMP for MMU systems, which means
we need to call arch_cpu_up() and arch_cpu_up_finish(). These two functions call
update_identity_mapping(). Since we believe "identity mapping" is a MMU concept,
we changed this to generic name "mm mapping” as arch_cpu_up() and 
arch_cpu_up_finish() is a shared path between MMU and MPU.

But I think MPU won’t use update_mm_mapping() function at all, so I wonder do
you prefer creating an empty stub update_identity_mapping() for MPU? Or use #ifdef
as suggested in your previous email...

> 
> If you don't want to keep update_identity_mapping(), then I would consider to simply wrap...

…here and ...

> 
>> +{
>> +    update_identity_mapping(enable);
>> +}
>> +
>>  extern void switch_ttbr_id(uint64_t ttbr);
>>    typedef void (switch_ttbr_fn)(uint64_t ttbr);
>> @@ -131,7 +136,7 @@ void __init switch_ttbr(uint64_t ttbr)
>>      lpae_t pte;
>>        /* Enable the identity mapping in the boot page tables */
>> -    update_identity_mapping(true);
>> +    update_mm_mapping(true);
>>        /* Enable the identity mapping in the runtime page tables */
>>      pte = pte_of_xenaddr((vaddr_t)switch_ttbr_id);
>> @@ -148,7 +153,7 @@ void __init switch_ttbr(uint64_t ttbr)
>>       * Note it is not necessary to disable it in the boot page tables
>>       * because they are not going to be used by this CPU anymore.
>>       */
>> -    update_identity_mapping(false);
>> +    update_mm_mapping(false);
>>  }
>>    /*
>> diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
>> index 9637f42469..2b1d086a1e 100644
>> --- a/xen/arch/arm/arm64/smpboot.c
>> +++ b/xen/arch/arm/arm64/smpboot.c
>> @@ -111,18 +111,18 @@ int arch_cpu_up(int cpu)
>>      if ( !smp_enable_ops[cpu].prepare_cpu )
>>          return -ENODEV;
>>  -    update_identity_mapping(true);
>> +    update_mm_mapping(true);
> 
> ... with #ifdef CONFIG_MMU here...
> 
>>        rc = smp_enable_ops[cpu].prepare_cpu(cpu);
>>      if ( rc )
>> -        update_identity_mapping(false);
>> +        update_mm_mapping(false);
> 
> ... here and ...
> 
> 
>>        return rc;
>>  }
>>    void arch_cpu_up_finish(void)
>>  {
>> -    update_identity_mapping(false);
>> +    update_mm_mapping(false);
> 
> ... here.

…all here?

Kind regards,
Henry
Julien Grall Aug. 24, 2023, 10:19 a.m. UTC | #3
Hi Henry,

On 24/08/2023 10:46, Henry Wang wrote:
>> On Aug 22, 2023, at 02:32, Julien Grall <julien@xen.org> wrote:
>>
>> Hi,
>>
>> On 14/08/2023 05:25, Henry Wang wrote:
>>> From: Penny Zheng <penny.zheng@arm.com>
>>> As preparation for MPU support, which will use some variables/functions
>>> for both MMU and MPU system, We rename the affected variable/function
>>> to more generic names:
>>> - init_ttbr -> init_mm,
>>
>> You moved init_ttbr to mm/mmu.c. So why does this need to be renamed?
>>
>> And if you really planned to use it for the MPU code. Then init_ttbr should not have been moved.
> 
> You are correct. I think we need to use the “init_mm” for MPU SMP support,
> so I would not move this variable in v6.

Your branch mpu_v5 doesn't seem to contain any use. But I would expect 
that the common is never going to use the variable. Also, at the moment 
it is 64-bit but I don't see why it would be necessary to be bigger than 
32-bit on 32-bit.

So I think it would be preferable if init_ttbr is move in mm/mmu.c. You
can then introduce an MPU specific variable.

In general, only variables that will be used by common code should be 
defined in common. All the rest should be defined in their specific 
directory.

>>> - mmu_init_secondary_cpu() -> mm_init_secondary_cpu()
>>> - init_secondary_pagetables() -> init_secondary_mm()
>>
>> The original naming were not great but the new one are a lot more confusing as they seem to just be a reshuffle of word.
>>
>> mm_init_secondary_cpu() is only setting the WxN bit. For the MMU, I think it can be done much earlier. Do you have anything to add in it? If not, then I would consider to get rid of it.
> 
> I’ve got rid of mmu_init_secondary_cpu() function in my local v6 as it is now
> folded to the assembly code.
> 
>>
>> For init_secondary_mm(), I would renamed it to prepare_secondary_mm().
> 
> Sure, thanks for the name suggestion.
> 
>>
>>>   -void update_identity_mapping(bool enable)
>>> +static void update_identity_mapping(bool enable)
>>
>> Why not simply renaming this function to update_mm_mapping()? But...
>>
>>>   {
>>>       paddr_t id_addr = virt_to_maddr(_start);
>>>       int rc;
>>> @@ -120,6 +120,11 @@ void update_identity_mapping(bool enable)
>>>       BUG_ON(rc);
>>>   }
>>>   +void update_mm_mapping(bool enable)
>>
>> ... the new name it quite confusing. What is the mapping it is referring to?
> 
> So I checked the MPU SMP support code and now I think I understand the reason
> why update_mm_mapping() was introduced:
> 
> In the future we eventually need to support SMP for MMU systems, which means
> we need to call arch_cpu_up() and arch_cpu_up_finish(). These two functions call
> update_identity_mapping(). Since we believe "identity mapping" is a MMU concept,
> we changed this to generic name "mm mapping” as arch_cpu_up() and
> arch_cpu_up_finish() is a shared path between MMU and MPU.

The function is today called "update_identity_mapping()" because this is 
what the implementation does on arm64. But the goal of this function is 
to make sure that any mapping necessary for bring-up secondary CPUs are 
present.

So if you don't need similar work for the MPU then I would go with...

> 
> But I think MPU won’t use update_mm_mapping() function at all, so I wonder do
> you prefer creating an empty stub update_identity_mapping() for MPU? Or use #ifdef
> as suggested in your previous email...


... #ifdef. I have some preliminary work where the call to 
update_identity_mapping() may end up to be moved somewhere else as the 
page-tables would not be shared between pCPU anymore. So the logic will 
not some rework (see [1]).

Cheers,


[1] https://lore.kernel.org/all/20221216114853.8227-21-julien@xen.org/
Henry Wang Aug. 24, 2023, 11:18 a.m. UTC | #4
Hi Julienm

> On Aug 24, 2023, at 18:19, Julien Grall <julien@xen.org> wrote:
> 
> Hi Henry,
> 
> On 24/08/2023 10:46, Henry Wang wrote:
>>> On Aug 22, 2023, at 02:32, Julien Grall <julien@xen.org> wrote:
>>> 
>>> Hi,
>>> 
>>> On 14/08/2023 05:25, Henry Wang wrote:
>>>> From: Penny Zheng <penny.zheng@arm.com>
>>>> As preparation for MPU support, which will use some variables/functions
>>>> for both MMU and MPU system, We rename the affected variable/function
>>>> to more generic names:
>>>> - init_ttbr -> init_mm,
>>> 
>>> You moved init_ttbr to mm/mmu.c. So why does this need to be renamed?
>>> 
>>> And if you really planned to use it for the MPU code. Then init_ttbr should not have been moved.
>> You are correct. I think we need to use the “init_mm” for MPU SMP support,
>> so I would not move this variable in v6.
> 
> Your branch mpu_v5 doesn't seem to contain any use. But I would expect that the common is never going to use the variable. Also, at the moment it is 64-bit but I don't see why it would be necessary to be bigger than 32-bit on 32-bit.
> 
> So I think it would be preferable if init_ttbr is move in mm/mmu.c. You
> can then introduce an MPU specific variable.

Sounds good to me.

> 
> In general, only variables that will be used by common code should be defined in common. All the rest should be defined in their specific directory.

Got it :))

> 
>>>> - mmu_init_secondary_cpu() -> mm_init_secondary_cpu()
>>>> - init_secondary_pagetables() -> init_secondary_mm()
>>> 
>>> The original naming were not great but the new one are a lot more confusing as they seem to just be a reshuffle of word.
>>> 
>>> mm_init_secondary_cpu() is only setting the WxN bit. For the MMU, I think it can be done much earlier. Do you have anything to add in it? If not, then I would consider to get rid of it.
>> I’ve got rid of mmu_init_secondary_cpu() function in my local v6 as it is now
>> folded to the assembly code.
>>> 
>>> For init_secondary_mm(), I would renamed it to prepare_secondary_mm().
>> Sure, thanks for the name suggestion.
>>> 
>>>>  -void update_identity_mapping(bool enable)
>>>> +static void update_identity_mapping(bool enable)
>>> 
>>> Why not simply renaming this function to update_mm_mapping()? But...
>>> 
>>>>  {
>>>>      paddr_t id_addr = virt_to_maddr(_start);
>>>>      int rc;
>>>> @@ -120,6 +120,11 @@ void update_identity_mapping(bool enable)
>>>>      BUG_ON(rc);
>>>>  }
>>>>  +void update_mm_mapping(bool enable)
>>> 
>>> ... the new name it quite confusing. What is the mapping it is referring to?
>> So I checked the MPU SMP support code and now I think I understand the reason
>> why update_mm_mapping() was introduced:
>> In the future we eventually need to support SMP for MMU systems, which means
>> we need to call arch_cpu_up() and arch_cpu_up_finish(). These two functions call
>> update_identity_mapping(). Since we believe "identity mapping" is a MMU concept,
>> we changed this to generic name "mm mapping” as arch_cpu_up() and
>> arch_cpu_up_finish() is a shared path between MMU and MPU.
> 
> The function is today called "update_identity_mapping()" because this is what the implementation does on arm64. But the goal of this function is to make sure that any mapping necessary for bring-up secondary CPUs are present.
> 
> So if you don't need similar work for the MPU then I would go with...
> 
>> But I think MPU won’t use update_mm_mapping() function at all, so I wonder do
>> you prefer creating an empty stub update_identity_mapping() for MPU? Or use #ifdef
>> as suggested in your previous email...
> 
> 
> ... #ifdef. I have some preliminary work where the call to update_identity_mapping() may end up to be moved somewhere else as the page-tables would not be shared between pCPU anymore. So the logic will not some rework (see [1]).

Thanks for sharing this info, I will drop the modification to update_identity_mapping()
from this patch.

Kind regards,
Henry


> 
> Cheers,
> 
> 
> [1] https://lore.kernel.org/all/20221216114853.8227-21-julien@xen.org/
> 
> -- 
> Julien Grall
diff mbox series

Patch

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 33b038e7e0..03ab68578a 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -238,11 +238,11 @@  GLOBAL(init_secondary)
 secondary_switched:
         /*
          * Non-boot CPUs need to move on to the proper pagetables, which were
-         * setup in init_secondary_pagetables.
+         * setup in init_secondary_mm.
          *
          * XXX: This is not compliant with the Arm Arm.
          */
-        mov_w r4, init_ttbr          /* VA of HTTBR value stashed by CPU 0 */
+        mov_w r4, init_mm            /* VA of HTTBR value stashed by CPU 0 */
         ldrd  r4, r5, [r4]           /* Actual value */
         dsb
         mcrr  CP64(r4, r5, HTTBR)
diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
index ba2ddd7e67..58d91c9088 100644
--- a/xen/arch/arm/arm64/mmu/head.S
+++ b/xen/arch/arm/arm64/mmu/head.S
@@ -302,7 +302,7 @@  ENDPROC(enable_mmu)
 ENTRY(enable_secondary_cpu_mm)
         mov   x5, lr
 
-        load_paddr x0, init_ttbr
+        load_paddr x0, init_mm
         ldr   x0, [x0]
 
         bl    enable_mmu
diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c
index 78b7c7eb00..ed0fc5ff7b 100644
--- a/xen/arch/arm/arm64/mmu/mm.c
+++ b/xen/arch/arm/arm64/mmu/mm.c
@@ -106,7 +106,7 @@  void __init arch_setup_page_tables(void)
     prepare_runtime_identity_mapping();
 }
 
-void update_identity_mapping(bool enable)
+static void update_identity_mapping(bool enable)
 {
     paddr_t id_addr = virt_to_maddr(_start);
     int rc;
@@ -120,6 +120,11 @@  void update_identity_mapping(bool enable)
     BUG_ON(rc);
 }
 
+void update_mm_mapping(bool enable)
+{
+    update_identity_mapping(enable);
+}
+
 extern void switch_ttbr_id(uint64_t ttbr);
 
 typedef void (switch_ttbr_fn)(uint64_t ttbr);
@@ -131,7 +136,7 @@  void __init switch_ttbr(uint64_t ttbr)
     lpae_t pte;
 
     /* Enable the identity mapping in the boot page tables */
-    update_identity_mapping(true);
+    update_mm_mapping(true);
 
     /* Enable the identity mapping in the runtime page tables */
     pte = pte_of_xenaddr((vaddr_t)switch_ttbr_id);
@@ -148,7 +153,7 @@  void __init switch_ttbr(uint64_t ttbr)
      * Note it is not necessary to disable it in the boot page tables
      * because they are not going to be used by this CPU anymore.
      */
-    update_identity_mapping(false);
+    update_mm_mapping(false);
 }
 
 /*
diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
index 9637f42469..2b1d086a1e 100644
--- a/xen/arch/arm/arm64/smpboot.c
+++ b/xen/arch/arm/arm64/smpboot.c
@@ -111,18 +111,18 @@  int arch_cpu_up(int cpu)
     if ( !smp_enable_ops[cpu].prepare_cpu )
         return -ENODEV;
 
-    update_identity_mapping(true);
+    update_mm_mapping(true);
 
     rc = smp_enable_ops[cpu].prepare_cpu(cpu);
     if ( rc )
-        update_identity_mapping(false);
+        update_mm_mapping(false);
 
     return rc;
 }
 
 void arch_cpu_up_finish(void)
 {
-    update_identity_mapping(false);
+    update_mm_mapping(false);
 }
 
 /*
diff --git a/xen/arch/arm/include/asm/arm64/mm.h b/xen/arch/arm/include/asm/arm64/mm.h
index e0bd23a6ed..7a389c4b21 100644
--- a/xen/arch/arm/include/asm/arm64/mm.h
+++ b/xen/arch/arm/include/asm/arm64/mm.h
@@ -15,13 +15,14 @@  static inline bool arch_mfns_in_directmap(unsigned long mfn, unsigned long nr)
 void arch_setup_page_tables(void);
 
 /*
- * Enable/disable the identity mapping in the live page-tables (i.e.
- * the one pointed by TTBR_EL2).
+ * In MMU system, enable/disable the identity mapping in the live
+ * page-tables (i.e. the one pointed by TTBR_EL2) through
+ * update_identity_mapping().
  *
  * Note that nested call (e.g. enable=true, enable=true) is not
  * supported.
  */
-void update_identity_mapping(bool enable);
+void update_mm_mapping(bool enable);
 
 #endif /* __ARM_ARM64_MM_H__ */
 
diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index dc1458b047..8084c62c01 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -170,7 +170,7 @@  struct page_info
 #define PGC_need_scrub    PGC_allocated
 
 /* Non-boot CPUs use this to find the correct pagetables. */
-extern uint64_t init_ttbr;
+extern uint64_t init_mm;
 
 #ifdef CONFIG_ARM_32
 #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page))
@@ -205,11 +205,13 @@  extern void setup_pagetables(unsigned long boot_phys_offset);
 extern void *early_fdt_map(paddr_t fdt_paddr);
 /* Remove early mappings */
 extern void remove_early_mappings(void);
-/* Allocate and initialise pagetables for a secondary CPU. Sets init_ttbr to the
- * new page table */
-extern int init_secondary_pagetables(int cpu);
+/*
+ * Allocate and initialise pagetables for a secondary CPU. Sets init_mm to the
+ * new page table
+ */
+extern int init_secondary_mm(int cpu);
 /* Switch secondary CPUS to its own pagetables and finalise MMU setup */
-extern void mmu_init_secondary_cpu(void);
+extern void mm_init_secondary_cpu(void);
 /* Map a frame table to cover physical addresses ps through pe */
 extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
 /* map a physical range in virtual memory */
diff --git a/xen/arch/arm/mmu/mm.c b/xen/arch/arm/mmu/mm.c
index 1d6267e6c5..7486c35ec0 100644
--- a/xen/arch/arm/mmu/mm.c
+++ b/xen/arch/arm/mmu/mm.c
@@ -106,7 +106,7 @@  DEFINE_BOOT_PAGE_TABLE(xen_fixmap);
 static DEFINE_PAGE_TABLES(xen_xenmap, XEN_NR_ENTRIES(2));
 
 /* Non-boot CPUs use this to find the correct pagetables. */
-uint64_t init_ttbr;
+uint64_t init_mm;
 
 static paddr_t phys_offset;
 
@@ -492,18 +492,18 @@  static void clear_boot_pagetables(void)
 }
 
 #ifdef CONFIG_ARM_64
-int init_secondary_pagetables(int cpu)
+int init_secondary_mm(int cpu)
 {
     clear_boot_pagetables();
 
-    /* Set init_ttbr for this CPU coming up. All CPus share a single setof
+    /* Set init_mm for this CPU coming up. All CPus share a single setof
      * pagetables, but rewrite it each time for consistency with 32 bit. */
-    init_ttbr = (uintptr_t) xen_pgtable + phys_offset;
-    clean_dcache(init_ttbr);
+    init_mm = (uintptr_t) xen_pgtable + phys_offset;
+    clean_dcache(init_mm);
     return 0;
 }
 #else
-int init_secondary_pagetables(int cpu)
+int init_secondary_mm(int cpu)
 {
     lpae_t *first;
 
@@ -529,16 +529,16 @@  int init_secondary_pagetables(int cpu)
 
     clear_boot_pagetables();
 
-    /* Set init_ttbr for this CPU coming up */
-    init_ttbr = __pa(first);
-    clean_dcache(init_ttbr);
+    /* Set init_mm for this CPU coming up */
+    init_mm = __pa(first);
+    clean_dcache(init_mm);
 
     return 0;
 }
 #endif
 
 /* MMU setup for secondary CPUS (which already have paging enabled) */
-void mmu_init_secondary_cpu(void)
+void mm_init_secondary_cpu(void)
 {
     xen_pt_enforce_wnx();
 }
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index e107b86b7b..8bcdbea66c 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -359,7 +359,7 @@  void start_secondary(void)
      */
     update_system_features(&current_cpu_data);
 
-    mmu_init_secondary_cpu();
+    mm_init_secondary_cpu();
 
     gic_init_secondary_cpu();
 
@@ -448,7 +448,7 @@  int __cpu_up(unsigned int cpu)
 
     printk("Bringing up CPU%d\n", cpu);
 
-    rc = init_secondary_pagetables(cpu);
+    rc = init_secondary_mm(cpu);
     if ( rc < 0 )
         return rc;