Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85568?usp=email )
Change subject: cpu/intel/car/romstage: Fix false-positive stack corruption ......................................................................
cpu/intel/car/romstage: Fix false-positive stack corruption
Fix regression introduced in commit 03518727313119a8c7a273eb745663c99f8e4d22 ("arch/x86: Add breakpoint to stack canary").
romstage_main writes to the stack-canary, but since that's expected temporarily disable the breakpoint. This only caused a warning on platforms that do select IDT_IN_EVERY_STAGE, since those install the stack canary breakpoint.
TEST: No more exceptions are printed in romstage when IDT_IN_EVERY_STAGE is enabled.
Change-Id: I7ebf0a5e8eaad49af77ab4d5f6b58fc849013b14 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/85568 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Shuo Liu shuo.liu@intel.com Reviewed-by: Lean Sheng Tan sheng.tan@9elements.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/cpu/intel/car/romstage.c 1 file changed, 18 insertions(+), 0 deletions(-)
Approvals: Angel Pons: Looks good to me, approved Shuo Liu: Looks good to me, approved build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, approved
diff --git a/src/cpu/intel/car/romstage.c b/src/cpu/intel/car/romstage.c index 7df512d..63a83ab 100644 --- a/src/cpu/intel/car/romstage.c +++ b/src/cpu/intel/car/romstage.c @@ -3,6 +3,7 @@ #include <adainit.h> #include <arch/romstage.h> #include <arch/symbols.h> +#include <arch/stack_canary_breakpoint.h> #include <commonlib/helpers.h> #include <console/console.h> #include <cpu/x86/smm.h> @@ -35,9 +36,26 @@
stack_base = (u32 *)(_ecar_stack - size);
+ /* Disable breakpoint since stack is intentionally corrupted */ + stack_canary_breakpoint_remove(); + + /* + * The "stack guard" and the "stack canary breakpoint" can both detect excessive + * stack usage. Excessive stack usage can corrupt data and lead to undefined + * (and thus hard to debug) behaviour. + * The stack guard will be checked later on, assuming the corruption wasn't to + * severe and allowed romstage to run. It's useful to detect problems when + * HW breakpoints were disabled. + * + * When HW breakpoints are used and enabled, the stack canary breakpoint will + * report the instruction pointer immediately, which can hint at which function + * may be using too much stack. FSP might disable HW breakpoints, though. + */ for (i = 0; i < num_guards; i++) stack_base[i] = stack_guard;
+ stack_canary_breakpoint_init(); + if (CONFIG(VBOOT_EARLY_EC_SYNC)) vboot_sync_ec();