Attention is currently required from: Arthur Heymans, Felix Singer, Tim Wawrzynczak, Angel Pons, Patrick Rudolph, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60405 )
Change subject: soc/intel/common/cse: Implement HECI notify ......................................................................
Patch Set 8:
(1 comment)
File src/soc/intel/common/block/cse/cse_eop.c:
https://review.coreboot.org/c/coreboot/+/60405/comment/a13103d4_1ae132fe PS7, Line 279: HeciEnabled
Ah, the HECI device can be enabled and initialised in coreboot, but hidden before booting an OS. So, if I understand correctly, there are multiple cases:
Yes, you can assume that HECI is default enabled and you can't disable the HECI in the regular operational mode. The only possibility is to make it function disable prior booting to the OS.
- HECI device disabled: I think this happens when CSE is in an abnormal state (e.g. because of some error or because it has been disabled). This situation should be detected at runtime and dealt with accordingly.
Yes, Mostly like EOP failure scenarios.
- HECI device enabled, and left visible: This is the simplest thing to do when the CSE is running normally, but I'm not sure if there are any drawbacks. I know Windows Device Manager shows a "yellow bang" for the HECI device when its drivers aren't installed, but it's not a serious problem.
You've correctly pointed out. When you don't have a HECI driver installed then ideally it's like Yellow bang or in linux, it tries to bind a generic PCI driver. There was an issue reported starting with SKL or KBL (I forgot the platform name) where the system was unable to enter into S0ix due to HECI. The debug data we had at that time was pointing to any access to HECI device PCI configuration space causing the non-entering to S0ix issue. Later we had to find a way to make HECI function disable when we don't have a driver in Chrome OS. This was the case mostly till CML, hence, you may find most of the Chrome platform prior to TGL had HeciEnabled set to `0` and post TGL it's set to `1` . Now I don't think we need to use `HeciEnabled` for any platform going forward.
- HECI device enabled, but hidden before booting an OS: I think this is to avoid the Windows Device Manager "yellow bang" thing when the drivers aren't installed, and I think it also saves a bit of power by keeping the HECI device in D0i3. Other than that, I'm not sure if there are any other reasons to do this.
Yes, I have mentioned the issue in detail, why we had to come up with HeciEnabled config variable.
Case 1 would be handled as part of the CSE error handling flows, so we don't need to worry about it in this commit. Cases 2 and 3 are with CSE running normally. This makes me wonder: are there any reasons to prefer case 2 over case 3, or viceversa? If so, which ones?
I hope now you have context as well, reading the issue descriptor above.
If there's no (mainboard-specific) reason for a particular mainboard to require either case 2 or case 3, I was thinking of having a user-configurable Kconfig option to control hiding the HECI device. This still allows mainboards to override the default setting, and also allows non-mainboard code to do so. For instance, if Chrome OS needs case 3, one would only need to override the default once with `if CHROMEOS` or similar.
I think we can take a few actions as below based on our discussion 1. Retire HeciEnabled from all SOC and MB.2. Replace the functionality that HeciEnable was achieving with DT CSE PCI device on/off. Typically this can manage Case #2 above.3. SoC code to make CSE hidden (almost case #3 above) when both DT CSE PCI device is off and the user selects HECI_DISABLE_USING_SMM config.This can also provide flexibility in the hand of mb user to choose if they wish to pick between case #2 or #3. WDYT?
Thoughts?