From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.9342.1681766316801381617 for ; Mon, 17 Apr 2023 14:18:36 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=qc8/EkUc; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 45ECD62AA4 for ; Mon, 17 Apr 2023 21:18:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0D2E5C433D2 for ; Mon, 17 Apr 2023 21:18:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1681766316; bh=8Svf5GVcwDIslfi9zN1vXRKu1yuN4O6av/h44SDJnsE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=qc8/EkUcS3ZKP5h8QbZUSRhkr28qohYpIILHoLSTK9n2dT8h3mxjYrWFh8xyo0sZC dvd8eMAab5jK7xPqILfiCecgf5wBff1NcHSMdtdFAlmLyLuu74mNVWF5fKeXczpMLl z42Q9yavsOiTZM8ffSUesFzzHTENlMtwKmU7ZYZGObMWpIIzxgndi8eWSu6bEUwmVl OhhbP4XES0DDKzFDtJZycIMnNo5OsmS7mtauPEcr1a5YIA3dU81IdqSGxZuQI77mBA Slp9eCxf0qRppybEIw2iyU14i06BgXvntJwlMD1t+hfri0zaNQa5u4+ySCLvgFkJ5y /fhXYIz/06wSg== Received: by mail-lf1-f42.google.com with SMTP id 2adb3069b0e04-4ec816d64afso8111210e87.1 for ; Mon, 17 Apr 2023 14:18:35 -0700 (PDT) X-Gm-Message-State: AAQBX9eCKfmT9SRstQuzxt8MSmrQgPqKRylUqHom7CJsbV9c+ZxRTel+ fHullFfyXMHFXlJS+dI4WN7tlqDoq6k+coiG3Hk= X-Google-Smtp-Source: AKy350YrIQhpdCNz67SxI/Hn8OYKTyVmlVifmuwbRLOVQ9I/16Ryb01xHpjTA4fFLDPdgGtVxGYay05U1UiGZNr79Ng= X-Received: by 2002:ac2:4e0c:0:b0:4d8:86c1:4784 with SMTP id e12-20020ac24e0c000000b004d886c14784mr2286945lfr.25.1681766314075; Mon, 17 Apr 2023 14:18:34 -0700 (PDT) MIME-Version: 1.0 References: <20230417180916.95237-1-mhaeuser@posteo.de> In-Reply-To: From: "Ard Biesheuvel" Date: Mon, 17 Apr 2023 23:18:22 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/2] ArmPkg/AsmMacroIoLibV8: Introduce ASM_FUNC_ALIGN() To: Leif Lindholm Cc: =?UTF-8?Q?Marvin_Ha=CC=88user?= , devel@edk2.groups.io, Ard Biesheuvel , Sami Mujawar , Vitaly Cheptsov Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 17 Apr 2023 at 21:52, Leif Lindholm wro= te: > > 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 Ha=CC=88user 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 afte= r > > the macro invocation lets the function label point to the location prio= r > > to alignment. Depending on toolchain behaviour, this may cause the labe= l > > 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=C3=A4user > > Cc: Leif Lindholm > > Cc: Ard Biesheuvel > > Cc: Sami Mujawar > > Cc: Vitaly Cheptsov > > --- > > ArmPkg/Include/AsmMacroIoLibV8.h | 22 ++++++++++++++++++---- > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/ArmPkg/Include/AsmMacroIoLibV8.h b/ArmPkg/Include/AsmMacro= IoLibV8.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 :-)