[S] Change in coreboot[master]: soc/intel/cmn/cpu: Add function to disable 3-strike CATERR
Attention is currently required from: Tarun Tuli, Subrata Banik, Paul Menzel, Ivy Jian, Angel Pons, Ronak Kanabar.
Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74158 )
Change subject: soc/intel/cmn/cpu: Add function to disable 3-strike CATERR ......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/74158/comment/e02a41d3_8a98503a PS3, Line 8:
I would directly write the whole thing (makes it easier to understand for me personally): […]
``` I'm not here to explain what 3-stick error mean/does. One can refer to the appropriate document for that. ``` Than refer to it. ``` I'm intended to enable a bit which is described in the EDS. I don't think for enabling the bit related to 3-strike, I need to write whole paragraph about what is 3-strike error. ``` Than write down the source of your information. ``` If I have to do a PCI BAR programming does that mean, I have to explain what is PCI and it's internal details ? ``` Thats a poor comparison. PCI is a very broad and very well known subject in Firmware/Coreboot community. It is referenced in numerous coreboot patches and can therefore be seen as "common knowledge". 3-three CATEER is a very specific thing that I assume is not known to "everyone" (at the very least its not known to me).
``` I have wrote the commit msg to explain what this patch does ``` Specifically I was missing the key information why disabling 3-three CATEER helps to do CPU traces. You say it does but you are not explaining why. At least one sentence about that would be enough for me. So I basically have to trust, that what you say is true or google for half an hour to check -> review time is much longer
Maybe my suggestion was a bit overkill, but I don't see the harm in adding it. Better to have more information than less.
-
Maximilian Brune (Code Review)