Attention is currently required from: Jason Nien, Martin Roth, Tim Van Patten.
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72455 )
Change subject: mb/google/skyrim: Add EC_HOST_EVENT_PANIC to SCI mask ......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/72455/comment/a74ebc2a_2695b498 PS1, Line 10: If system safe mode is also enabled : on the EC, the kernel will have a short period to extract and save info : about the EC panic.
I'm not sure how EC panics are handled today, so I've got a few questions: […]
1. It is not enabled in EC by default. It needs to be enabled with Kconfig flag. See https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4195658.
2. This change https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4063818 causes the EC to publish the event when a panic occurs (this is independent of system safe mode). Without system safe mode, the EC just unilaterally resets the system when a panic occurs. So with this change an SCI will interrupt AP/OS but it will be reset almost immediately. I don't expect this results in any new risks that aren't already present.
With system safe mode enabled, the EC will resume for 2 seconds after a panic. Without additional kernel driver changes, nothing is handling this event, so still no impact.
With https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/4... the kernel driver will handle the EC panic event. It attempts to flush the EC log and then triggers an OS reset after 1 second.
3. EC resets 2 seconds after panic, OS resets 1 second after receiving EC panic event. We could extend this if needed.
https://review.coreboot.org/c/coreboot/+/72455/comment/6bf80731_35a98d5b PS1, Line 16: TEST=Observe kernel ec panic handler run when ec panics
Please also validate things behave correctly with the EC feature disabled, (presumably) meaning the […]
Sure. I'll enumerate the scenarios to be tested, let me know if I missed any: 1. No Safe Mode + No Kernel Handler 2. Safe Mode + No Kernel Handler 3. No Safe Mode + Kernel Handler 4. Safe Mode + Kernel Handler
-
Rob Barnes (Code Review)