On Mon, 17 Apr 2023 at 21:52, Leif Lindholm <quic_llind...@quicinc.com> wrote: > > Hi Marvin, > > First of all - many thanks for tracking down the bug that creates the > need for this. > > On Mon, Apr 17, 2023 at 18:09:15 +0000, Marvin Häuser wrote: > > With the current ASM_FUNC() macro, there is no good way to declare an > > alignment constraint for a function. As ASM_FUNC() switches sections, > > declaring the constraint before the macro invocation applies it to the > > current location in the previous section. Declaring the constraint after > > the macro invocation lets the function label point to the location prior > > to alignment. Depending on toolchain behaviour, this may cause the label > > to point to alignment padding preceding the actual function definition. > > > > To address these issues, introduce the ASM_FUNC_ALIGN() macro, which > > declares the alignment constraint right before the function label. > > > > Signed-off-by: Marvin Häuser <mhaeu...@posteo.de> > > Cc: Leif Lindholm <quic_llind...@quicinc.com> > > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org> > > Cc: Sami Mujawar <sami.muja...@arm.com> > > Cc: Vitaly Cheptsov <vit9...@protonmail.com> > > --- > > ArmPkg/Include/AsmMacroIoLibV8.h | 22 ++++++++++++++++++---- > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/ArmPkg/Include/AsmMacroIoLibV8.h > > b/ArmPkg/Include/AsmMacroIoLibV8.h > > index 135aaeca5d0b..919edc70384d 100644 > > --- a/ArmPkg/Include/AsmMacroIoLibV8.h > > +++ b/ArmPkg/Include/AsmMacroIoLibV8.h > > @@ -34,15 +34,29 @@ > > cbnz SAFE_XREG, 1f ;\ > > b . ;// We should never get here > > > > -#define _ASM_FUNC(Name, Section) \ > > - .global Name ; \ > > - .section #Section, "ax" ; \ > > - .type Name, %function ; \ > > +#define _ASM_FUNC_HDR(Name, Section) \ > > + .global Name ; \ > > + .section #Section, "ax" ; \ > > + .type Name, %function > > + > > +#define _ASM_FUNC_FTR(Name) \ > > Name: ; \ > > AARCH64_BTI(c) > > > > +#define _ASM_FUNC(Name, Section) \ > > + _ASM_FUNC_HDR(Name, Section) ; \ > > + _ASM_FUNC_FTR(Name) > > + > > +#define _ASM_FUNC_ALIGN(Name, Section, Align) \ > > I like this solution, but I'd like to hear Ard's opinion. > > I probably want to bikeshed some of the implementation details: > Although I generally dislike duplicate definitions, I think I would > prefer having _ASM_FUNC and _ASM_FUNC_ALIGN defined self-contained, > without _HDR and _FTR. > If we do keep the reused primitives, we need better language; the > footer of the header is not a footer of the function. >
Agree with all of this. And thanks for tracking this down - must not have been fun :-) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103121): https://edk2.groups.io/g/devel/message/103121 Mute This Topic: https://groups.io/mt/98325898/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
