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

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi, @Michal Orzel

On Mon, Jun 2, 2025 at 10:10 AM Orzel, Michal <michal.orzel@xxxxxxx> wrote:
>
>
>
> On 30/05/2025 22:31, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> >
> > 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@xxxxxxxx>
> > ---
> >  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@xxxxxxx>

Thank you for reviewing the patch.

>
> ~Michal
>

~Mykola