Re: [Xen-devel] [PATCH 10/17] xen/arm64: head: Improve coding style and document create_pages_tables()

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 26/06/2019 02:03, Stefano Stabellini wrote:
> On Mon, 10 Jun 2019, Julien Grall wrote:
> > Adjust the coding style used in the comments within create_pages_tables()
> >
> > Lastly, document the behavior and the main registers usage within the
> > function. Note that x25 is now only used within the function, so it does
> > not need to be part of the common register.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > ---
> >   xen/arch/arm/arm64/head.S | 34 +++++++++++++++++++++++-----------
> >   1 file changed, 23 insertions(+), 11 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index ee0024173e..7b92c1c8eb 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -70,7 +70,7 @@
> >    *  x22 - is_secondary_cpu
> >    *  x23 - UART address
> >    *  x24 -
> > - *  x25 - identity map in place
> > + *  x25 -
> >    *  x26 - skip_zero_bss (boot cpu only)
> >    *  x27 -
> >    *  x28 -
> > @@ -443,16 +443,27 @@ cpu_init:
> >           ret
> >   ENDPROC(cpu_init)
> >   
> > +/*
> > + * Rebuild the boot pagetable's first-level entries. The structure
> > + * is described in mm.c.
> > + *
> > + * After the CPU enables paging it will add the fixmap mapping
> > + * to these page tables, however this may clash with the 1:1
> > + * mapping. So each CPU must rebuild the page tables here with
> > + * the 1:1 in place.
> > + *
> > + * Inputs:
> > + *   x19: paddr(start)
> > + *   x20: phys offset
> Is x20 actually used?
Yes via the macro load_paddr.

> The rest looks fine.
>
>
> > + * Clobbers x0 - x4, x25
> > + *
> > + * Register usage within this function:
> > + *   x25: Identity map in place
> > + */
> >   create_page_tables:
> > -        /* Rebuild the boot pagetable's first-level entries. The structure
> > -         * is described in mm.c.
> > -         *
> > -         * After the CPU enables paging it will add the fixmap mapping
> > -         * to these page tables, however this may clash with the 1:1
> > -         * mapping. So each CPU must rebuild the page tables here with
> > -         * the 1:1 in place. */
> > -
> > -        /* If Xen is loaded at exactly XEN_VIRT_START then we don't
> > +        /*
> > +         * If Xen is loaded at exactly XEN_VIRT_START then we don't
> >            * need an additional 1:1 mapping, the virtual mapping will
> >            * suffice.
> >            */
> > @@ -476,7 +487,8 @@ create_page_tables:
> >           cbz   x1, 1f                 /* It's in slot 0, map in boot_first
> >                                         * or boot_second later on */
> >   
> > -        /* Level zero does not support superpage mappings, so we have
> > +        /*
> > +         * Level zero does not support superpage mappings, so we have
> >            * to use an extra first level page in which we create a 1GB mapping.
> >            */
> >           load_paddr x2, boot_first_id
> > --
> > 2.11.0
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel