[PATCH] xen/arm64: head: document missing input registers for MMU functions

Mykola Kvach posted 1 patch 8 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/9b1f50a40e3634f859ad8e7446c24e43caaa38eb.1748637004.git.mykola._5Fkvach@epam.com
There is a newer version of this series
xen/arch/arm/arm64/mmu/head.S | 3 +++
1 file changed, 3 insertions(+)
[PATCH] xen/arm64: head: document missing input registers for MMU functions
Posted by Mykola Kvach 8 months, 2 weeks ago
From: Mykola Kvach <mykola_kvach@epam.com>

Add missing input register descriptions to enable_secondary_cpu_mm
and enable_boot_cpu_mm functions.

Specifically:
- x19 is used in enable_boot_cpu_mm as physical start address.
- x20 is used in both functions for physical offset passed to load_paddr.

This update improves code clarity and consistency in comments.

No functional changes are introduced by this patch.

Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
 xen/arch/arm/arm64/mmu/head.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
index 634156f83d..033b3a018a 100644
--- a/xen/arch/arm/arm64/mmu/head.S
+++ b/xen/arch/arm/arm64/mmu/head.S
@@ -313,6 +313,7 @@ END(enable_mmu)
  *
  * Inputs:
  *   lr : Virtual address to return to.
+ *   x20: phys offset (used by load_paddr)
  *
  * Clobbers x0 - x6
  */
@@ -337,6 +338,8 @@ END(enable_secondary_cpu_mm)
  *
  * Inputs:
  *   lr : Virtual address to return to.
+ *   x19: paddr(start) (used by remove_identity_mapping)
+ *   x20: phys offset (used by load_paddr)
  *
  * Clobbers x0 - x6
  */
-- 
2.48.1
Re: [PATCH] xen/arm64: head: document missing input registers for MMU functions
Posted by Orzel, Michal 8 months, 1 week ago

On 30/05/2025 22:31, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@epam.com>
> 
> Add missing input register descriptions to enable_secondary_cpu_mm
> and enable_boot_cpu_mm functions.
> 
> Specifically:
> - x19 is used in enable_boot_cpu_mm as physical start address.
> - x20 is used in both functions for physical offset passed to load_paddr.
I'm not sure if we need to document register usage that is part of a comment in
so called "Common register usage". It's repeating information for me. That said,
I can see that Arm32 does that too so no objection.

> 
> This update improves code clarity and consistency in comments.
> 
> No functional changes are introduced by this patch.
> 
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
>  xen/arch/arm/arm64/mmu/head.S | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
> index 634156f83d..033b3a018a 100644
> --- a/xen/arch/arm/arm64/mmu/head.S
> +++ b/xen/arch/arm/arm64/mmu/head.S
> @@ -313,6 +313,7 @@ END(enable_mmu)
>   *
>   * Inputs:
>   *   lr : Virtual address to return to.
> + *   x20: phys offset (used by load_paddr)
>   *
>   * Clobbers x0 - x6
>   */
> @@ -337,6 +338,8 @@ END(enable_secondary_cpu_mm)
>   *
>   * Inputs:
>   *   lr : Virtual address to return to.
> + *   x19: paddr(start) (used by remove_identity_mapping)
AFAICT it's also used by create_page_tables. I don't see it beneficial to
mention the places the register is used. It can easily go stale.

With the remark addressed:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal
Re: [PATCH] xen/arm64: head: document missing input registers for MMU functions
Posted by Mykola Kvach 8 months, 1 week ago
Hi, @Michal Orzel

On Mon, Jun 2, 2025 at 10:10 AM Orzel, Michal <michal.orzel@amd.com> wrote:
>
>
>
> On 30/05/2025 22:31, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > Add missing input register descriptions to enable_secondary_cpu_mm
> > and enable_boot_cpu_mm functions.
> >
> > Specifically:
> > - x19 is used in enable_boot_cpu_mm as physical start address.
> > - x20 is used in both functions for physical offset passed to load_paddr.
> I'm not sure if we need to document register usage that is part of a comment in
> so called "Common register usage". It's repeating information for me. That said,
> I can see that Arm32 does that too so no objection.

I'll drop that sentence from the commit message.

>
> >
> > This update improves code clarity and consistency in comments.
> >
> > No functional changes are introduced by this patch.
> >
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> >  xen/arch/arm/arm64/mmu/head.S | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
> > index 634156f83d..033b3a018a 100644
> > --- a/xen/arch/arm/arm64/mmu/head.S
> > +++ b/xen/arch/arm/arm64/mmu/head.S
> > @@ -313,6 +313,7 @@ END(enable_mmu)
> >   *
> >   * Inputs:
> >   *   lr : Virtual address to return to.
> > + *   x20: phys offset (used by load_paddr)
> >   *
> >   * Clobbers x0 - x6
> >   */
> > @@ -337,6 +338,8 @@ END(enable_secondary_cpu_mm)
> >   *
> >   * Inputs:
> >   *   lr : Virtual address to return to.
> > + *   x19: paddr(start) (used by remove_identity_mapping)
> AFAICT it's also used by create_page_tables. I don't see it beneficial to
> mention the places the register is used. It can easily go stale.

I'll remove the mention of usage locations.

>
> With the remark addressed:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thank you for reviewing the patch.

>
> ~Michal
>

~Mykola