Re: [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap()

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 6/26/19 8:01 PM, Stefano Stabellini wrote:
> On Mon, 10 Jun 2019, Julien Grall wrote:
> > At the moment, the fixmap table is only hooked when earlyprintk is used.
> > This is fine today because in C land, the fixmap is not used by anyone
> > until the the boot CPU is switching to the runtime page-tables.
> >
> > In the future, the boot CPU will not switch between page-tables to avoid
> > TLB conflict. This means the fixmap table will need to be hooked before
> > any use. For simplicity, setup_fixmap() will now do that job.
> Can I ask you to reword this slightly, especially the last sentence? It
> took me a while to understand what you meant. I suggest:
>
>   In the future, the boot CPU will not switch between page-tables to
>   avoid any TLB conflicts. Thus, the fixmap table will need to be always
>   hooked before any use. Let's start doing it now in setup_fixmap().
I will update the commit message.

> Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> >
> > Lastly, document the behavior and the main registers usage within the
> > function.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > ---
> >   xen/arch/arm/arm64/head.S | 13 +++++++++++--
> >   1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index 96e85f8834..4f7fa6769f 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -700,8 +700,17 @@ id_map_removed:
> >           ret
> >   ENDPROC(remove_id_map)
> >   
> > +/*
> > + * Map the UART in the fixmap (when earlyprintk is used) and hook the
> > + * fixmap table in the page tables.
> > + *
> > + * The fixmap cannot be mapped in create_page_tables because it may
> > + * clash with the ID map.
> > + *
> > + * Clobbers x0 - x1
> > + */
> >   setup_fixmap:
> > -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
> > +#ifdef CONFIG_EARLY_PRINTK
> I am curious why you made this code style change
This is the only #if defined within head.S (the rest use #ifdef) so for 
consistency. Also, it is simpler to read :).
Cheers,

--
Julien Grall
I

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