[PATCH v2 4/5] lib: sbi: Create a spot to place Smrnmi detection before traps and after DT is ready

[Evgeny Voevodin]

Hi Anup,

Two concerns about the suggested approach.

1) Enablement of NMIs before the platform has set up its NMI handler
introduces a window of possible hang. While NMIE=1, the trap handler is
not defined, so if an NMI arrives, the trap jumps to nowhere.

The v2 series keeps NMIE=0 until smrnmi_handlers_init() has installed
both NMIVEC and the regular trap entry, which avoids this hole.

More generally: on Smrnmi-enabled platforms, there is really no way to
use any trap-based mechanism until the platform has set its NMI handlers.
Holding off them until after a call to the platform's
smrnmi_handlers_init() is mandatory to prevent possible hangs.

2) About the stack canary concern with the C-side __stack_chk_guard_init().

The danger pattern "canary already pushed on stack" only triggers if a
function call which returns is currently on stack, which means
__stack_chk_guard variable will be checked against its value on prologue.
Neither C nor asm functions currently on stack return or use epilogue
check of __stack_chk_guard. The full call chain to __stack_chk_guard_init()
consists of:

  asm _start
   -> sbi_init()              __noreturn
      -> init_coldboot()      __noreturn
         -> __stack_chk_guard_init()    asm, no canary

Even if asm side falls back to `return`, it is still `j _start_hang`
which also doesn't check epilogue.

Could you reconsider patch 4/5 with this context?

Thanks,
Evgeny