Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46354 )
Change subject: include/cpu/x86: introduce new helper for (un)setting MSRs ......................................................................
Patch Set 3: Code-Review+2
(5 comments)
Thanks!
https://review.coreboot.org/c/coreboot/+/46354/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46354/3//COMMIT_MSG@10 PS3, Line 10: MRS MSR
https://review.coreboot.org/c/coreboot/+/46354/3//COMMIT_MSG@12 PS3, Line 12: CB:42134 nit, the commit hash is easier to look up and set in stone. I use `git log -1 --format='%h (%s)'` usually. If you say 'commit' before it, it's also a link in Gerrit iirc.
https://review.coreboot.org/c/coreboot/+/46354/3/src/cpu/intel/haswell/final... File src/cpu/intel/haswell/finalize.c:
https://review.coreboot.org/c/coreboot/+/46354/3/src/cpu/intel/haswell/final... PS3, Line 10: (1 << 0) nit, the comma has highest precedence anyway (no need for parantheses around arguments)
https://review.coreboot.org/c/coreboot/+/46354/3/src/include/cpu/x86/msr.h File src/include/cpu/x86/msr.h:
https://review.coreboot.org/c/coreboot/+/46354/3/src/include/cpu/x86/msr.h@3... PS3, Line 319: } We could also add an msr_set(reg, set) shorthand for the `unset == 0` case.
https://review.coreboot.org/c/coreboot/+/46354/3/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/msr.h:
https://review.coreboot.org/c/coreboot/+/46354/3/src/soc/intel/common/block/... PS3, Line 59: _BIT nit, I'd drop the _BIT suffix
-
Nico Huber (Code Review)