* [PATCH v3 1/2] ArmPkg/AsmMacroIoLibV8: Introduce ASM_FUNC_ALIGN()
@ 2023-04-20 15:24 Marvin Häuser
2023-04-20 15:24 ` [PATCH v3 2/2] ArmPkg/ArmMmuLib: Fix ArmReplaceLiveTranslationEntry() alignment Marvin Häuser
0 siblings, 1 reply; 3+ messages in thread
From: Marvin Häuser @ 2023-04-20 15:24 UTC (permalink / raw)
To: devel
Cc: Marvin Häuser, Leif Lindholm, Ard Biesheuvel, Sami Mujawar,
Vitaly Cheptsov
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 <mhaeuser@posteo.de>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
---
ArmPkg/Include/AsmMacroIoLibV8.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/ArmPkg/Include/AsmMacroIoLibV8.h b/ArmPkg/Include/AsmMacroIoLibV8.h
index 135aaeca5d0b..81164ea9c9e7 100644
--- a/ArmPkg/Include/AsmMacroIoLibV8.h
+++ b/ArmPkg/Include/AsmMacroIoLibV8.h
@@ -41,8 +41,19 @@
Name: ; \
AARCH64_BTI(c)
+#define _ASM_FUNC_ALIGN(Name, Section, Align) \
+ .global Name ; \
+ .section #Section, "ax" ; \
+ .type Name, %function ; \
+ .balign Align ; \
+ Name: ; \
+ AARCH64_BTI(c)
+
#define ASM_FUNC(Name) _ASM_FUNC(ASM_PFX(Name), .text. ## Name)
+#define ASM_FUNC_ALIGN(Name, Align) \
+ _ASM_FUNC_ALIGN(ASM_PFX(Name), .text. ## Name, Align)
+
#define MOV32(Reg, Val) \
movz Reg, (Val) >> 16, lsl #16 ; \
movk Reg, (Val) & 0xffff
--
2.40.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v3 2/2] ArmPkg/ArmMmuLib: Fix ArmReplaceLiveTranslationEntry() alignment
2023-04-20 15:24 [PATCH v3 1/2] ArmPkg/AsmMacroIoLibV8: Introduce ASM_FUNC_ALIGN() Marvin Häuser
@ 2023-04-20 15:24 ` Marvin Häuser
2023-04-20 15:58 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Marvin Häuser @ 2023-04-20 15:24 UTC (permalink / raw)
To: devel
Cc: Marvin Häuser, Leif Lindholm, Ard Biesheuvel, Sami Mujawar,
Vitaly Cheptsov
As the ASM_FUNC() macro performs a section switch, the preceding
.balign directive applies the alignment constraint to the current
location in the previous section. As the linker may not merge the
sections in-order, ArmReplaceLiveTranslationEntry() may be left
unaligned.
Replace the explicit invocation of .balign with the ASM_FUNC_ALIGN()
macro, which guarantees the alignment constraint is applied correctly.
To make sure related issues are reliably caught in the future, align the
end of the function before checking the total occupied size. This
ensures crossing a 0x200 boundary will cause a compilation error.
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
---
.../ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
index e936a5be4e11..887439bc042f 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
@@ -69,17 +69,16 @@
.L2_\@:
.endm
- // Align this routine to a log2 upper bound of its size, so that it is
- // guaranteed not to cross a page or block boundary.
- .balign 0x200
-
//VOID
//ArmReplaceLiveTranslationEntry (
// IN UINT64 *Entry,
// IN UINT64 Value,
// IN UINT64 Address
// )
-ASM_FUNC(ArmReplaceLiveTranslationEntry)
+//
+// Align this routine to a log2 upper bound of its size, so that it is
+// guaranteed not to cross a page or block boundary.
+ASM_FUNC_ALIGN(ArmReplaceLiveTranslationEntry, 0x200)
// disable interrupts
mrs x4, daif
@@ -101,5 +100,8 @@ ASM_GLOBAL ASM_PFX(ArmReplaceLiveTranslationEntrySize)
ASM_PFX(ArmReplaceLiveTranslationEntrySize):
.long . - ArmReplaceLiveTranslationEntry
- // Double check that we did not overrun the assumed maximum size
+ // Double check that we did not overrun the assumed maximum size or cross a
+ // 0x200 boundary (and thus implicitly not any larger power of two, including
+ // the page size).
+ .balign 0x200
.org ArmReplaceLiveTranslationEntry + 0x200
--
2.40.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3 2/2] ArmPkg/ArmMmuLib: Fix ArmReplaceLiveTranslationEntry() alignment
2023-04-20 15:24 ` [PATCH v3 2/2] ArmPkg/ArmMmuLib: Fix ArmReplaceLiveTranslationEntry() alignment Marvin Häuser
@ 2023-04-20 15:58 ` Ard Biesheuvel
0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2023-04-20 15:58 UTC (permalink / raw)
To: Marvin Häuser
Cc: devel, Leif Lindholm, Ard Biesheuvel, Sami Mujawar,
Vitaly Cheptsov
On Thu, 20 Apr 2023 at 17:24, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
> As the ASM_FUNC() macro performs a section switch, the preceding
> .balign directive applies the alignment constraint to the current
> location in the previous section. As the linker may not merge the
> sections in-order, ArmReplaceLiveTranslationEntry() may be left
> unaligned.
>
> Replace the explicit invocation of .balign with the ASM_FUNC_ALIGN()
> macro, which guarantees the alignment constraint is applied correctly.
> To make sure related issues are reliably caught in the future, align the
> end of the function before checking the total occupied size. This
> ensures crossing a 0x200 boundary will cause a compilation error.
>
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Thanks. I've queued these up as #4291
> ---
> .../ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> index e936a5be4e11..887439bc042f 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> @@ -69,17 +69,16 @@
> .L2_\@:
> .endm
>
> - // Align this routine to a log2 upper bound of its size, so that it is
> - // guaranteed not to cross a page or block boundary.
> - .balign 0x200
> -
> //VOID
> //ArmReplaceLiveTranslationEntry (
> // IN UINT64 *Entry,
> // IN UINT64 Value,
> // IN UINT64 Address
> // )
> -ASM_FUNC(ArmReplaceLiveTranslationEntry)
> +//
> +// Align this routine to a log2 upper bound of its size, so that it is
> +// guaranteed not to cross a page or block boundary.
> +ASM_FUNC_ALIGN(ArmReplaceLiveTranslationEntry, 0x200)
>
> // disable interrupts
> mrs x4, daif
> @@ -101,5 +100,8 @@ ASM_GLOBAL ASM_PFX(ArmReplaceLiveTranslationEntrySize)
> ASM_PFX(ArmReplaceLiveTranslationEntrySize):
> .long . - ArmReplaceLiveTranslationEntry
>
> - // Double check that we did not overrun the assumed maximum size
> + // Double check that we did not overrun the assumed maximum size or cross a
> + // 0x200 boundary (and thus implicitly not any larger power of two, including
> + // the page size).
> + .balign 0x200
> .org ArmReplaceLiveTranslationEntry + 0x200
> --
> 2.40.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-20 15:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-20 15:24 [PATCH v3 1/2] ArmPkg/AsmMacroIoLibV8: Introduce ASM_FUNC_ALIGN() Marvin Häuser
2023-04-20 15:24 ` [PATCH v3 2/2] ArmPkg/ArmMmuLib: Fix ArmReplaceLiveTranslationEntry() alignment Marvin Häuser
2023-04-20 15:58 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox