| Message ID | 20190422164937.21350-13-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Clean-up & fixes in boot/mm code | expand |
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@arm.com> > --- > 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? > 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@epam.com>
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@arm.com> >> --- >> 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@epam.com> Thank you. Cheers,
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 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 strd r2, r3, [r1, r4] /* Map it in the fixmap's slot */ /* Use a virtual address to access the UART. */
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@arm.com> --- xen/arch/arm/arm32/head.S | 3 +++ 1 file changed, 3 insertions(+)