Hi Julien, On Fri, Apr 22, 2016 at 05:08:26PM +0100, Julien Grall wrote: >On 21/04/16 02:06, Peng Fan wrote: >>Hi Julien, > >Hello Peng, > >>On Wed, Apr 20, 2016 at 03:44:09PM +0100, Julien Grall wrote: >>>Hello Peng, >>> >>>On 20/04/16 14:54, Peng Fan wrote: >>>>Use shift operator, but not muliplication. >>>>No function change. >>> >>>Why? The compiler will calculate the address at compilation time. >> >>Yeah. The compiler will do this. I just think in asm, we rarely use >>multiplication. In this file, there is "cmp x1, #(LPAE_ENTRIES<<3)", >>here use shift operator but not multiplication. > >It took me a bit of time to understand that 8 is the number of byte of an >LPAE entries. IMHO the "<< 3" would be more confusing. > >So I would introduce a macro to get the slot based of an index. Something >like: > >LPAE_SLOT(slot) ((slot) * 8) > >And add a comment to explain why 8. You could even introduce LPAE_ENTRY_SIZE.
Thanks for comments. I'll try to submit a new patch following your suggestion. Thanks, Peng. > >Regards, > >>Thanks, >>Peng. >> >>> >>>Regards, >>> >>>>Signed-off-by: Peng Fan <van.free...@gmail.com> >>>>Cc: Stefano Stabellini <sstabell...@kernel.org> >>>>Cc: Julien Grall <julien.gr...@arm.com> >>>>--- >>>> xen/arch/arm/arm64/head.S | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>>diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >>>>index 05e3db0..ad6e593 100644 >>>>--- a/xen/arch/arm/arm64/head.S >>>>+++ b/xen/arch/arm/arm64/head.S >>>>@@ -524,7 +524,7 @@ paging: >>>> lsl x2, x2, #THIRD_SHIFT /* 4K aligned paddr of UART */ >>>> mov x3, #PT_DEV_L3 >>>> orr x2, x2, x3 /* x2 := 4K dev map including UART */ >>>>- str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first >>>>fixmap's slot */ >>>>+ str x2, [x1, #(FIXMAP_CONSOLE << 3)] /* Map it in the first >>>>fixmap's slot */ >>>> 1: >>>> >>>> /* Map fixmap into boot_second */ >>>> >>> >>>-- >>>Julien Grall >> > >-- >Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
