Fix the wrong return value of both InternalSyncIncrement() and InternalSyncDecrement(). The return value shouldn't be the address of input parameter. It should be the updated value of input parameter instead.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org --- MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S b/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S index ecb87fc..830ea5b 100644 --- a/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S +++ b/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S @@ -171,6 +171,7 @@ TryInternalSyncIncrement: add w1, w1, #1 stxr w2, w1, [x0] cbnz w2, TryInternalSyncIncrement + mov x0, x1 dmb sy ret
@@ -199,5 +200,6 @@ TryInternalSyncDecrement: sub w1, w1, #1 stxr w2, w1, [x0] cbnz w2, TryInternalSyncDecrement + mov x0, x1 dmb sy ret
Hi Haojian,
On 27 October 2015 at 16:15, Haojian Zhuang haojian.zhuang@linaro.org wrote:
Fix the wrong return value of both InternalSyncIncrement() and InternalSyncDecrement(). The return value shouldn't be the address of input parameter. It should be the updated value of input parameter instead.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org
MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S b/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S index ecb87fc..830ea5b 100644 --- a/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S +++ b/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S @@ -171,6 +171,7 @@ TryInternalSyncIncrement: add w1, w1, #1 stxr w2, w1, [x0] cbnz w2, TryInternalSyncIncrement
- mov x0, x1
Thanks for fixing this. However, I prefer we use 'mov w0, w1' here, even though the result will be the same.
With that change: Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
(I think we can fix it up when applying, so no need to resend. Leif?)
On Tue, Oct 27, 2015 at 06:15:07PM +0900, Ard Biesheuvel wrote:
Hi Haojian,
On 27 October 2015 at 16:15, Haojian Zhuang haojian.zhuang@linaro.org wrote:
Fix the wrong return value of both InternalSyncIncrement() and InternalSyncDecrement(). The return value shouldn't be the address of input parameter. It should be the updated value of input parameter instead.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org
MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S b/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S index ecb87fc..830ea5b 100644 --- a/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S +++ b/MdePkg/Library/BaseSynchronizationLib/AArch64/Synchronization.S @@ -171,6 +171,7 @@ TryInternalSyncIncrement: add w1, w1, #1 stxr w2, w1, [x0] cbnz w2, TryInternalSyncIncrement
- mov x0, x1
Thanks for fixing this. However, I prefer we use 'mov w0, w1' here, even though the result will be the same.
With that change: Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
(I think we can fix it up when applying, so no need to resend. Leif?)
Agreed - fixed up and committed to SVN as r18685. (Also added "AArch64" to subject.)
This closes the issue identified by Tyler Smith back in March: http://edk2-devel.narkive.com/8mRclrSE/patch-armpkg-fix-arm-assembly-for-bas...
Thanks!
/ Leif
-- Ard.
dmb sy ret
@@ -199,5 +200,6 @@ TryInternalSyncDecrement: sub w1, w1, #1 stxr w2, w1, [x0] cbnz w2, TryInternalSyncDecrement
- mov x0, x1 dmb sy ret
-- 1.9.1
-
Ard Biesheuvel -
Haojian Zhuang -
Leif Lindholm