Hi Nate,
Firstly, thanks!

I'll respond to these comments here, and the rest on the main patch.

1. I've added it to my to-do list

2. We could also consider calling 
https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchPolicyLib/PchSamplePreMemPolicyLib.c
 and 
https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchPolicyLib/PchSamplePolicyLib.c
 ( 
https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchPolicyLib/PchSamplePolicyLib.c,
 ) , the same way that a sample policy is called for SA at 
https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Library/PeiSaPolicyLib/PeiSaPolicyLib.c#L372.
 I'm also aware that some of this file duplicates a *PolicyLib, but I never got 
around to following the specific policy entries being created or removing lines 
from this file and testing.

2(a). I've dropped the HPET and IO APIC BDF settings, and the board still 
boots. I think I added these when it wasn't booting, so I tried to align as 
closely with what I thought was correct. They never got dropped, I guess.

2(b). Okay, I can drop this line when that patch is merged. I don't think it's 
crucial for boot, though.

2(c). I'll try to do that, but I can also drop these lines until I get to it.

2(d). Where do you see that? Yes, the deepest the system has gone is package 
C10. However, the FSP default for PchPmSlpS0Enable=1 is preserved. I don't know 
what support this feature depends on, so I don't know if the board is missing 
it. If it's only S0ix, perhaps it should be enabled and it may work once I 
figure out what needs to be power-gated. Of course, there could be 
board-specific blockers: the WLAN doesn't support ASPM L0s.

2(e)-(i). Done.

3. This definition is used by BaseEcLib at 
https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseEcLib/BaseEcLib.c#L35.
 While this command is never issued, a definition is needed to be able to use 
the library.

4(a)-(b). Done. I chose EcSendTime() instead.

4(c). I've implemented this function, which is now moved to DxeBoardInitLib. 
Currently, I've only reverse engineered this function's HW interaction, not the 
context in which the vendor firmware calls it's protocol.

4(d). Done.

5. I know. Are you responding to a previous comment I had here about enabling 
modules, writing libraries and enabling the CSM? Currently, my change here is 
to PcdSiCatalogDebugEnable, which is conditionally set for 
https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Intel/KabylakeSiliconPkg/SiPkgBuildOption.dsc#L43-L45
 to allow me to have release-optimised images with debug printing enabled. I 
may be appropriating this PCD, as I think we discussed, but it has no other 
purpose in the edk2-platforms code.

I've also addressed your comment on VBT location.

Regards,
Benjamin


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79512): https://edk2.groups.io/g/devel/message/79512
Mute This Topic: https://groups.io/mt/84876362/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to