Re: [Xen-devel] [PATCH 12/20] xen/arm32: head: Always zero r3 before update a page-table entry

[Julien Grall <julien.grall@xxxxxxx>]

On 03/05/2019 16:57, Andrii Anisov wrote:
> Hello Julien,
>
> On 22.04.19 19:49, Julien Grall wrote:
> > The boot code is using r2 and r3 to hold the page-table entry value.
> > While r2 is always updated before storing the value, this is not always
> > the case for r3.
> >
> > Thankfully today, r3 will always be zero when we care. But this is
> > difficult to track and error-prone.
> >
> > So always zero r3 within the few instructions before the write the
> > page-table entry.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > ---
> >   xen/arch/arm/arm32/head.S | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index 3448817aab..0536b62aec 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -270,6 +270,7 @@ cpu_init_done:
> >           orr   r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */
> >           orr   r2, r2, #PT_LOWER(MEM)
> >           lsl   r1, r1, #3             /* r1 := Slot offset */
> > +        mov   r3, #0x0>           strd  r2, r3, [r4, r1]       /* Mapping of 
> > paddr(start) */
> >           mov   r6, #1                 /* r6 := identity map now in place */
> > @@ -377,6 +378,7 @@ paging:
> >           lsl   r2, r2, #THIRD_SHIFT   /* 4K aligned paddr of UART */
> >           orr   r2, r2, #PT_UPPER(DEV_L3)
> >           orr   r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including 
> > UART */
> > +        mov   r3, #0
> What's the difference between `#0x0` and `#0`? I've seen the usage is mixed 
> across the file, but not sure why. Could it be unified?
No difference, matter of taste. The file seems to use 0x0 in more places, so I 
will use 0x0 in this patch as well.
> >           strd  r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first 
> > fixmap's slot */
> >   1:
> > @@ -388,6 +390,7 @@ paging:
> >           orr   r2, r2, #PT_LOWER(PT)  /* r2:r3 := table map of xen_fixmap */
> >           ldr   r4, =FIXMAP_ADDR(0)
> >           mov   r4, r4, lsr #(SECOND_SHIFT - 3)   /* r4 := Slot for FIXMAP(0) */
> > +        mov   r3, #0
> Ditto.
>
> >           strd  r2, r3, [r1, r4]       /* Map it in the fixmap's slot */
> >           /* Use a virtual address to access the UART. */
> With the minor comments:
>
> Reviewed-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
Thank you.

Cheers,

--
Julien Grall

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