| Message ID | 1425479396-26438-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 20 March 2015 at 13:00, Ronald Cron <Ronald.Cron@arm.com> wrote: > Hi Ard, I have one question about this patch. Why do you still save r7 into the stack as it is not modified anymore by the routine ? > To retain 8 byte alignment in the SP Regards, Ard. > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: 04 March 2015 14:30 > To: edk2-devel@lists.sourceforge.net; liming.gao@intel.com > Subject: [edk2] [PATCH] MdePkg: fix ARM version of InternalMathSwapBytes64 () > > The ARM asm implementation of InternalMathSwapBytes64 () does > interesting things if bit 7 of operand r1 (upper 32 bits of the > input value) is set. After the recursive swap, bit 7 ends up in > the sign bit position, after which it is right shifted with sign > extension, and or'ed with the upper half of the output value. > > This means SwapBytes64 (0x00000080_00000000) returns an incorrect > value of 0xFFFFFFFF_80000000. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > MdePkg/Library/BaseLib/Arm/Math64.S | 16 ++++------------ > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/MdePkg/Library/BaseLib/Arm/Math64.S b/MdePkg/Library/BaseLib/Arm/Math64.S > index 4d975739207a..e2512621fe1a 100755 > --- a/MdePkg/Library/BaseLib/Arm/Math64.S > +++ b/MdePkg/Library/BaseLib/Arm/Math64.S > @@ -256,22 +256,14 @@ L30: > GCC_ASM_EXPORT(InternalMathSwapBytes64) > > ASM_PFX(InternalMathSwapBytes64): > - @ args = 0, pretend = 0, frame = 0 > - @ frame_needed = 1, uses_anonymous_args = 0 > - stmfd sp!, {r4, r5, r6, r7, lr} > - add r7, sp, #12 > + stmfd sp!, {r4, r5, r7, lr} > mov r5, r1 > bl ASM_PFX(SwapBytes32) > - mov r6, r0 > + mov r4, r0 > mov r0, r5 > bl ASM_PFX(SwapBytes32) > - mov r4, r6 > - mov r5, r4, asr #31 > - mov r2, #0 > - mov r1, r0, asr #31 > - orr r0, r0, r2 > - orr r1, r1, r4 > - ldmfd sp!, {r4, r5, r6, r7, pc} > + mov r1, r4 > + ldmfd sp!, {r4, r5, r7, pc} > > > ASM_FUNCTION_REMOVE_IF_UNREFERENCED > \ No newline at end of file > -- > 1.8.3.2 > > > ------------------------------------------------------------------------------ > Dive into the World of Parallel Programming The Go Parallel Website, sponsored > by Intel and developed in partnership with Slashdot Media, is your hub for all > things parallel software development, from weekly thought leadership blogs to > news, videos, case studies, tutorials and more. Take a look and join the > conversation now. http://goparallel.sourceforge.net/ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 > > > ------------------------------------------------------------------------------ > Dive into the World of Parallel Programming The Go Parallel Website, sponsored > by Intel and developed in partnership with Slashdot Media, is your hub for all > things parallel software development, from weekly thought leadership blogs to > news, videos, case studies, tutorials and more. Take a look and join the > conversation now. http://goparallel.sourceforge.net/ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/
diff --git a/MdePkg/Library/BaseLib/Arm/Math64.S b/MdePkg/Library/BaseLib/Arm/Math64.S index 4d975739207a..e2512621fe1a 100755 --- a/MdePkg/Library/BaseLib/Arm/Math64.S +++ b/MdePkg/Library/BaseLib/Arm/Math64.S @@ -256,22 +256,14 @@ L30: GCC_ASM_EXPORT(InternalMathSwapBytes64) ASM_PFX(InternalMathSwapBytes64): - @ args = 0, pretend = 0, frame = 0 - @ frame_needed = 1, uses_anonymous_args = 0 - stmfd sp!, {r4, r5, r6, r7, lr} - add r7, sp, #12 + stmfd sp!, {r4, r5, r7, lr} mov r5, r1 bl ASM_PFX(SwapBytes32) - mov r6, r0 + mov r4, r0 mov r0, r5 bl ASM_PFX(SwapBytes32) - mov r4, r6 - mov r5, r4, asr #31 - mov r2, #0 - mov r1, r0, asr #31 - orr r0, r0, r2 - orr r1, r1, r4 - ldmfd sp!, {r4, r5, r6, r7, pc} + mov r1, r4 + ldmfd sp!, {r4, r5, r7, pc} ASM_FUNCTION_REMOVE_IF_UNREFERENCED
The ARM asm implementation of InternalMathSwapBytes64 () does interesting things if bit 7 of operand r1 (upper 32 bits of the input value) is set. After the recursive swap, bit 7 ends up in the sign bit position, after which it is right shifted with sign extension, and or'ed with the upper half of the output value. This means SwapBytes64 (0x00000080_00000000) returns an incorrect value of 0xFFFFFFFF_80000000. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- MdePkg/Library/BaseLib/Arm/Math64.S | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) \ No newline at end of file