Attention is currently required from: Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54092 )
Change subject: security/intel/txt: Split of microcode error type printing ......................................................................
Patch Set 3:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/54092/comment/240be392_eab01beb PS3, Line 7: of nit: `off` or `out`
File src/security/intel/txt/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/54092/comment/b03e454f_3fa4db16 PS3, Line 3: bootblock-y += logging.c This hunk shouldn't be part of this commit.
File src/security/intel/txt/logging.c:
https://review.coreboot.org/c/coreboot/+/54092/comment/2acb6810_f828366a PS3, Line 12: microcode Document 315168-017 calls these "processor-initiated Intel TXT shutdowns". I'd replace `microcode` with `processor`.
https://review.coreboot.org/c/coreboot/+/54092/comment/ad0d67cb_ad44fe9d PS3, Line 14: types Maybe call this `names` to avoid confusion with the `type` parameter?
https://review.coreboot.org/c/coreboot/+/54092/comment/d73e2c28_d92a11ac PS3, Line 31: }; There's a much better way to do this:
const char *intel_txt_processor_error_type(uint8_t type) { static const char *const names[] = { [0] = "Legacy Shutdown", [5] = "Load memory type error in ACM area", [6] = "Unrecognized ACM format", [7] = "Failure to authenticate", [8] = "Invalid ACM format", [9] = "Unexpected Snoop hit", [10] = "Invalid event", [11] = "Invalid MLE", [12] = "Machine check event", [13] = "VMXAbort", [14] = "AC memory corruption", [15] = "Illegal voltage/bus ratio", }; return type < ARRAY_SIZE(names) && names[type] ? names[type] : "Unknown"; }
This can be improved even further if one makes the function print the strings directly, and return nothing:
void intel_txt_log_processor_error(int loglevel, uint8_t type) { static const char *const names[] = { [0] = "Legacy Shutdown", [5] = "Load memory type error in ACM area", [6] = "Unrecognized ACM format", [7] = "Failure to authenticate", [8] = "Invalid ACM format", [9] = "Unexpected Snoop hit", [10] = "Invalid event", [11] = "Invalid MLE", [12] = "Machine check event", [13] = "VMXAbort", [14] = "AC memory corruption", [15] = "Illegal voltage/bus ratio", }; if (type < ARRAY_SIZE(names) && names[type]) printk(loglevel, "Error condition: %s\n", names[type]); else printk(loglevel, "Error condition: Unknown %u\n", type); }
https://review.coreboot.org/c/coreboot/+/54092/comment/5299c379_5898af47 PS3, Line 58: intel_txt_microcode_error_type For another patch: This shouldn't be called when the error cause is external.