Re: [Xen-devel] [PATCH v2 29/35] xen/arm32: head: Move assembly switch to the runtime PT in secondary CPUs path

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 7/30/19 9:25 PM, Stefano Stabellini wrote:
> On Mon, 22 Jul 2019, Julien Grall wrote:
> > The assembly switch to the runtime PT is only necessary for the
> > secondary CPUs. So move the code in the secondary CPUs path.
> >
> > While this is definitely not compliant with the Arm Arm as we are
> > switching between two differents set of page-tables without turning off
> > the MMU. Turning off the MMU is impossible here as the ID map may clash
> > with other mappings in the runtime page-tables. This will require more
> > rework to avoid the problem. So for now add a TODO in the code.
> >
> > Finally, the code is currently assume that r5 will be properly set to 0
> > before hand. This is done by create_page_tables() which is called quite
> > early in the boot process. There are a risk this may be oversight in the
> > future and therefore breaking secondary CPUs boot. Instead, set r5 to 0
> > just before using it.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> >
> > ---
> >      Changes in v2:
> >          - Patch added
> > ---
> >   xen/arch/arm/arm32/head.S | 42 ++++++++++++++++++++----------------------
> >   1 file changed, 20 insertions(+), 22 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index 4081a52dfa..6dc6032498 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -201,6 +201,26 @@ GLOBAL(init_secondary)
> >           mov   pc, r0
> >   secondary_switched:
> >           bl    setup_fixmap
> > +
> > +        /*
> > +         * Non-boot CPUs need to move on to the proper pagetables, which were
> > +         * setup in init_secondary_pagetables.
> > +         *
> > +         * XXX: This is not compliant with the Arm Arm.
> > +         */
> > +        ldr   r4, =init_ttbr         /* VA of HTTBR value stashed by CPU 0 */
> > +        mov   r5, #0
> Why do we need to zero r5? Shouldn't ldrd overwrite r5 anyway?
I was on auto-pilot mode and saw the trailing "d" and thought it was a 
"strd". I will remove it in the next version.
Cheers,

--
Julien Grall

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