Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Stefan Reinauer, Name of user not set #1003801, Fred Reitberger, Julian Schroeder, ron minnich, Felix Held. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63871 )
Change subject: src/lib: add interactive debug shell ......................................................................
Patch Set 2:
(13 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63871/comment/10484b32_903a17b4 PS2, Line 2: JulianMarcusSchroeder Please add spaces.
https://review.coreboot.org/c/coreboot/+/63871/comment/1021ce6d_f37bc83e PS2, Line 11: -stop Please add a space.
https://review.coreboot.org/c/coreboot/+/63871/comment/5dd51cc1_70785cb7 PS2, Line 14: -quicker reboot cycles: no need to boot into payload or OS Booting the payload shouldn’t make a lot of difference as quick as coreboot is. (Well below one second?)
https://review.coreboot.org/c/coreboot/+/63871/comment/fc288a0c_058811b3 PS2, Line 15: -ability to add tests: one build, multiple experiments can be run interactively Please elaborate.
https://review.coreboot.org/c/coreboot/+/63871/comment/342e4005_b37d1e96 PS2, Line 11: -stop system at various locations to preserve state (Smart Trace Buffer) : -abililty to probe system state interactively, i.e. communicate with service : processors, call specific functions : -quicker reboot cycles: no need to boot into payload or OS : -ability to add tests: one build, multiple experiments can be run interactively : -adding additonal test is simple. : -equivalent of 'BIOS Settings' can be added : -get boot time information (cbmem -t style) when serial console out is disabled Please reflow for 72 characters per line.
https://review.coreboot.org/c/coreboot/+/63871/comment/7a28d813_29abcc82 PS2, Line 23: JulianMarcusSchroeder Ditto.
Patchset:
PS2: Very nice, but no idea, if that aligns with coreboot’s philosophy. A lot of the use cases would also be solved by a separate payload.
File Documentation/debug/debug_shell.md:
https://review.coreboot.org/c/coreboot/+/63871/comment/f535a6ec_d2c5495d PS2, Line 1: #Debugging Please add spaces. Also below.
https://review.coreboot.org/c/coreboot/+/63871/comment/52eb9906_03901b41 PS2, Line 5: If one wants to benchmark a system's boot time up to the payload level, : there is no need to boot all the way to the OS. : If a system exhibits issues before reaching the payload level GDB or an : external debugger might be needed which isn't always practical or convenient. : Sometimes relevant system state is lost further into the boot process. : In addition, calling coreboot functions and utilities is easiest done within : coreboot. Please reflow. (Also below.)
https://review.coreboot.org/c/coreboot/+/63871/comment/529307e0_c5e72f4a PS2, Line 28: Maybe add one example with QEMU?
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/63871/comment/2b033957_0c435a93 PS2, Line 890: amd and
https://review.coreboot.org/c/coreboot/+/63871/comment/10dabd8b_501ca29c PS2, Line 891: A An
https://review.coreboot.org/c/coreboot/+/63871/comment/0f7aae52_981f68c0 PS2, Line 892: a an