* [PATCH v5 1/1] OvmfPkg/Sec: fix stack switch
@ 2022-06-08 10:09 Gerd Hoffmann
2022-06-08 12:23 ` Yao, Jiewen
0 siblings, 1 reply; 2+ messages in thread
From: Gerd Hoffmann @ 2022-06-08 10:09 UTC (permalink / raw)
To: devel
Cc: Oliver Steffen, James Bottomley, Liming Gao, Tom Lendacky,
Brijesh Singh, Gerd Hoffmann, Jordan Justen, Bob Feng, Yuwei Chen,
Min Xu, Erdem Aktas, Jiewen Yao, Ard Biesheuvel, Pawel Polawski,
Jiri Slaby
The ebp/rbp register can either be used for the frame pointer or
as general purpose register. With gcc (and clang) this depends
on the -f(no-)omit-frame-pointer switch.
This patch updates tools_def.template to explicitly set the compiler
option and also add a define to allow conditionally compile code.
The new define is used to fix stack switching in TemporaryRamMigration.
The ebp/rbp must not be touched when the compiler can use it as general
purpose register. With version 12 gcc starts actually using the
register, so changing it leads to firmware crashes in some
configurations.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3934
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/Sec/SecMain.c | 4 ++++
BaseTools/Conf/tools_def.template | 6 +++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 1167d22a68cc..3ca0dcdfd3dd 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -1052,11 +1052,15 @@ TemporaryRamMigration (
if (SetJump (&JumpBuffer) == 0) {
#if defined (MDE_CPU_IA32)
JumpBuffer.Esp = JumpBuffer.Esp + DebugAgentContext.StackMigrateOffset;
+ #ifndef OMIT_FRAME_POINTER
JumpBuffer.Ebp = JumpBuffer.Ebp + DebugAgentContext.StackMigrateOffset;
#endif
+ #endif
#if defined (MDE_CPU_X64)
JumpBuffer.Rsp = JumpBuffer.Rsp + DebugAgentContext.StackMigrateOffset;
+ #ifndef OMIT_FRAME_POINTER
JumpBuffer.Rbp = JumpBuffer.Rbp + DebugAgentContext.StackMigrateOffset;
+ #endif
#endif
LongJump (&JumpBuffer, (UINTN)-1);
}
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 5ed19810b727..adcd23f7273f 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -1849,9 +1849,9 @@ NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG = --add-gnu-debuglink=$(DEBUG_DIR)/$(MODULE_N
*_*_*_DTC_PATH = DEF(DTC_BIN)
DEFINE GCC_ALL_CC_FLAGS = -g -Os -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -include AutoGen.h -fno-common
-DEFINE GCC_IA32_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double -freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-probe
-DEFINE GCC_X64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone -Wno-address -mno-stack-arg-probe
-DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft -fno-pic -fno-pie
+DEFINE GCC_IA32_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double -freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-probe -fno-omit-frame-pointer
+DEFINE GCC_X64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone -Wno-address -mno-stack-arg-probe -fomit-frame-pointer -DOMIT_FRAME_POINTER=1
+DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-sections -fomit-frame-pointer -DOMIT_FRAME_POINTER=1 -Wno-address -mthumb -mfloat-abi=soft -fno-pic -fno-pie
DEFINE GCC_ARM_CC_XIPFLAGS = -mno-unaligned-access
DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -fno-short-enums -fverbose-asm -funsigned-char -ffunction-sections -fdata-sections -Wno-address -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-pic -fno-pie -ffixed-x18
DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align -mgeneral-regs-only
--
2.36.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v5 1/1] OvmfPkg/Sec: fix stack switch
2022-06-08 10:09 [PATCH v5 1/1] OvmfPkg/Sec: fix stack switch Gerd Hoffmann
@ 2022-06-08 12:23 ` Yao, Jiewen
0 siblings, 0 replies; 2+ messages in thread
From: Yao, Jiewen @ 2022-06-08 12:23 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io
Cc: Oliver Steffen, James Bottomley, Gao, Liming, Tom Lendacky,
Brijesh Singh, Justen, Jordan L, Feng, Bob C, Chen, Christine,
Xu, Min M, Aktas, Erdem, Ard Biesheuvel, Pawel Polawski,
Jiri Slaby
Merged https://github.com/tianocore/edk2/pull/2956
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Wednesday, June 8, 2022 6:10 PM
> To: devel@edk2.groups.io
> Cc: Oliver Steffen <osteffen@redhat.com>; James Bottomley
> <jejb@linux.ibm.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Tom
> Lendacky <thomas.lendacky@amd.com>; Brijesh Singh
> <brijesh.singh@amd.com>; Gerd Hoffmann <kraxel@redhat.com>; Justen,
> Jordan L <jordan.l.justen@intel.com>; Feng, Bob C <bob.c.feng@intel.com>;
> Chen, Christine <yuwei.chen@intel.com>; Xu, Min M <min.m.xu@intel.com>;
> Aktas, Erdem <erdemaktas@google.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Pawel
> Polawski <ppolawsk@redhat.com>; Jiri Slaby <jirislaby@kernel.org>
> Subject: [PATCH v5 1/1] OvmfPkg/Sec: fix stack switch
>
> The ebp/rbp register can either be used for the frame pointer or
> as general purpose register. With gcc (and clang) this depends
> on the -f(no-)omit-frame-pointer switch.
>
> This patch updates tools_def.template to explicitly set the compiler
> option and also add a define to allow conditionally compile code.
>
> The new define is used to fix stack switching in TemporaryRamMigration.
> The ebp/rbp must not be touched when the compiler can use it as general
> purpose register. With version 12 gcc starts actually using the
> register, so changing it leads to firmware crashes in some
> configurations.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3934
> Reported-by: Jiri Slaby <jirislaby@kernel.org>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> OvmfPkg/Sec/SecMain.c | 4 ++++
> BaseTools/Conf/tools_def.template | 6 +++---
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> index 1167d22a68cc..3ca0dcdfd3dd 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -1052,11 +1052,15 @@ TemporaryRamMigration (
> if (SetJump (&JumpBuffer) == 0) {
> #if defined (MDE_CPU_IA32)
> JumpBuffer.Esp = JumpBuffer.Esp + DebugAgentContext.StackMigrateOffset;
> + #ifndef OMIT_FRAME_POINTER
> JumpBuffer.Ebp = JumpBuffer.Ebp + DebugAgentContext.StackMigrateOffset;
> #endif
> + #endif
> #if defined (MDE_CPU_X64)
> JumpBuffer.Rsp = JumpBuffer.Rsp + DebugAgentContext.StackMigrateOffset;
> + #ifndef OMIT_FRAME_POINTER
> JumpBuffer.Rbp = JumpBuffer.Rbp + DebugAgentContext.StackMigrateOffset;
> + #endif
> #endif
> LongJump (&JumpBuffer, (UINTN)-1);
> }
> diff --git a/BaseTools/Conf/tools_def.template
> b/BaseTools/Conf/tools_def.template
> index 5ed19810b727..adcd23f7273f 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -1849,9 +1849,9 @@ NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG = --add-
> gnu-debuglink=$(DEBUG_DIR)/$(MODULE_N
> *_*_*_DTC_PATH = DEF(DTC_BIN)
>
> DEFINE GCC_ALL_CC_FLAGS = -g -Os -fshort-wchar -fno-builtin -fno-
> strict-aliasing -Wall -Werror -Wno-array-bounds -include AutoGen.h -fno-
> common
> -DEFINE GCC_IA32_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-
> double -freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-
> probe
> -DEFINE GCC_X64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone -
> Wno-address -mno-stack-arg-probe
> -DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian
> -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-
> sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft -fno-pic
> -fno-pie
> +DEFINE GCC_IA32_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-
> double -freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-
> probe -fno-omit-frame-pointer
> +DEFINE GCC_X64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone
> -Wno-address -mno-stack-arg-probe -fomit-frame-pointer -
> DOMIT_FRAME_POINTER=1
> +DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian
> -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-
> sections -fomit-frame-pointer -DOMIT_FRAME_POINTER=1 -Wno-address -
> mthumb -mfloat-abi=soft -fno-pic -fno-pie
> DEFINE GCC_ARM_CC_XIPFLAGS = -mno-unaligned-access
> DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-
> endian -fno-short-enums -fverbose-asm -funsigned-char -ffunction-sections -
> fdata-sections -Wno-address -fno-asynchronous-unwind-tables -fno-unwind-
> tables -fno-pic -fno-pie -ffixed-x18
> DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align -mgeneral-regs-only
> --
> 2.36.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-06-08 12:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-08 10:09 [PATCH v5 1/1] OvmfPkg/Sec: fix stack switch Gerd Hoffmann
2022-06-08 12:23 ` Yao, Jiewen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox