[edk2-devel] [edk2-platforms][PATCH v2 0/5] KabylakeOpenBoardPkg: Add AspireVn7Dash572G

[Benjamin Doron]

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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20210818/87408fc5/attachment.htm>