| Message ID | 1458220815-6944-2-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 21 March 2016 at 17:53, Cohen, Eugene <eugene@hp.com> wrote: > Alexei and Ard, > >> The suggested code for EL1_OR_EL2 macro: >> >> mrs SAFE_XREG, CurrentEL ;\ >> cmp SAFE_XREG, #0x8 ;\ >> b.eq 2f ;\ >> tbnz SAFE_XREG, #2, 1f ;\ >> b . ;// We should never get here >> >> will successfully branch to 1f label in case of EL3 with SAFE_XREG = 0xC >> because bit #2 will be set. > > Agreed. If you assume that EL1_OR_EL2 can never be invoked at EL3 then obviously this wouldn't be a concern. But since there's already a case for handling neither EL1 nor EL2 (b .) then it would make sense to correct this so if it was accidentally used in EL3 it would hit the default case which is better for debugability. The last tbnz needs to be reverted to the previous cmp/b.ne construct or equivalent. > Thanks for spotting that. I committed it as follows: // CurrentEL : 0xC = EL3; 8 = EL2; 4 = EL1 // This only selects between EL1 and EL2, else we die. // Provide the Macro with a safe temp xreg to use. #define EL1_OR_EL2(SAFE_XREG) \ mrs SAFE_XREG, CurrentEL ;\ cmp SAFE_XREG, #0x8 ;\ b.gt . ;\ b.eq 2f ;\ cbnz SAFE_XREG, 1f ;\ b . ;// We should never get here // CurrentEL : 0xC = EL3; 8 = EL2; 4 = EL1 // This only selects between EL1 and EL2 and EL3, else we die. // Provide the Macro with a safe temp xreg to use. #define EL1_OR_EL2_OR_EL3(SAFE_XREG) \ mrs SAFE_XREG, CurrentEL ;\ cmp SAFE_XREG, #0x8 ;\ b.gt 3f ;\ b.eq 2f ;\ cbnz SAFE_XREG, 1f ;\ b . ;// We should never get here _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Include/AsmMacroIoLibV8.h b/ArmPkg/Include/AsmMacroIoLibV8.h index a9f8491bc922..efc47d3bbbc7 100644 --- a/ArmPkg/Include/AsmMacroIoLibV8.h +++ b/ArmPkg/Include/AsmMacroIoLibV8.h @@ -25,9 +25,9 @@ mrs SAFE_XREG, CurrentEL ;\ cmp SAFE_XREG, #0x8 ;\ b.eq 2f ;\ - cmp SAFE_XREG, #0x4 ;\ - b.ne . ;// We should never get here -// EL1 code starts here + tbnz SAFE_XREG, #2, 1f ;\ + b . ;// We should never get here + // CurrentEL : 0xC = EL3; 8 = EL2; 4 = EL1 // This only selects between EL1 and EL2 and EL3, else we die. @@ -36,11 +36,10 @@ mrs SAFE_XREG, CurrentEL ;\ cmp SAFE_XREG, #0xC ;\ b.eq 3f ;\ - cmp SAFE_XREG, #0x8 ;\ - b.eq 2f ;\ - cmp SAFE_XREG, #0x4 ;\ - b.ne . ;// We should never get here -// EL1 code starts here + tbnz SAFE_XREG, #3, 2f ;\ + tbnz SAFE_XREG, #2, 1f ;\ + b . ;// We should never get here + #if defined(__clang__) // load x0 with _Data
The macros EL1_OR_EL2() and EL1_OR_EL2_OR_EL3() allow conditional execution of assembly sequences based on the current exception level, by jumping to caller supplied labels 1f, 2f or 3f. However, the jump to 1f is actually a fallthrough, which means the EL1 code needs to follow right after the macro invocation, and the 1f label is ignored. So let's fix this by making all jumps explicit. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Include/AsmMacroIoLibV8.h | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) -- 2.5.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel