Eugene, 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. Alexei. -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Cohen, Eugene Sent: 21 March 2016 15:29 To: Ard Biesheuvel; [email protected]; [email protected] Subject: Re: [edk2] [PATCH 1/7] ArmPkg/AsmMacroIoLibV8: remove undocumented assumption from ELx macros Thanks - I agree, the fallthrough was totally non-obvious and inconsistent with other cases. The whole '1f' syntax is pretty odd as well - I had to dig deep to find out what the locally defined numbered forward-only labels were. Reviewed-by: Eugene Cohen <[email protected]> > -----Original Message----- > From: Ard Biesheuvel [mailto:[email protected]] > Sent: Thursday, March 17, 2016 7:20 AM > To: [email protected]; [email protected]; Cohen, Eugene > <[email protected]> > Cc: Ard Biesheuvel <[email protected]> > Subject: [PATCH 1/7] ArmPkg/AsmMacroIoLibV8: remove undocumented > assumption from ELx macros > > 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 <[email protected]> > --- > ArmPkg/Include/AsmMacroIoLibV8.h | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > 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 > -- > 2.5.0 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/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. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
