Re: [PATCH 1/2] ArmPkg/AsmMacroIoLibV8: Introduce ASM_FUNC_ALIGN()

["Ard Biesheuvel" <ardb@kernel.org>]

On Tue, 18 Apr 2023 at 10:18, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
>
>
> > On 18. Apr 2023, at 10:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 18 Apr 2023 at 08:40, Marvin Häuser <mhaeuser@posteo.de> wrote:
> >>
> >>
> >>> On 17. Apr 2023, at 23:18, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>>
> >>> Agree with all of this.
> >>>
> >>> And thanks for tracking this down - must not have been fun :-)
> >>
> >> No worries - it wasn’t. :) It was mere luck Vitaly discovered early it was an issue that triggered based on the binary layout rather than a bug in the mapping code itself…
> >>
> >> Speaking of not fun to track down, I initially wanted to add ASSERTs (yes, runtime :( ) to check the alignment guarantee is actually met. Leif basically declined any form of regression-testing at runtime. Do you happen to have a simple(!) idea for how it could be checked at build-time? (It’s less about “which commands do I run?” and more about integration with the build process / BaseTools, cross-OS compatibility, etc.)
> >>
> >
> >
> > I think we should just add another align to the code:
> >
> > .align xx
> > func:
> >
> > < code >
> >
> > .align xx
> > .org func + xx
> >
> > That way, if the code straddles a xx-aligned boundary, the .org will
> > move backwards and force an error.
>
> Wow, that's pretty fucking smart... I didn't even know that directive was a thing. I will try to toy with it soon. Do you want it as a separate series, separate commit in the current series, or squashed into the fix?

I think it makes sense to just fold this into the second patch.