Re: 回复: 回复: [edk2-devel] [PATCH v2] UefiCpuPkg/MpInitLib: Add check for CR3/GDT/IDT.

[Laszlo Ersek]

On 09/04/20 13:48, Yao, Jiewen wrote:
> Jeff
> I think the CR3 should be managed by the CPU driver. And it should NOT be changed by other module during post phase without a clear contract.
> 
> Since CPU driver managed the value (or DebugAgent managing it under contract), it should *always* be valid. I don’t see the value to check the value managed by the module itself, and return EFI_UNSUPPORTED.
> 
> If CPU driver sets CR3 above 4G and sync it to AP, then it is a bug.
> We should fix the CPU driver or MP Library to *make it always work*.
> 
> There are multiple options. For example, the CPU driver can always set a dedicate CR3/GDT/IDT for AP (below 4G). Then no matter BSP state is changed, we can always wake up AP correctly with a pre-defined state. Syncing BSP state to AP just an implementation choice, it is not absolute necessary.
> 
> In a system boot, the ability to wake up AP is very important. Especially when the BIOS transfers the state to OS, the BSP need wake up AP to put AP in a good state, such as protected mode with paging disabled. Without doing that, we may get random system crash during boot.
> 
> If we really really want to do some check to ensure AP state is good, I would rather to prepare a dedicated known good state for AP and always use this known good state for AP wakeup.
> Giving up and returning UNSUPPORTED is definitely NOT my preference.

Should we audit the various AllocatePageTableMemory() function
implementations in edk2?

Or is there more stuff we need to look at?

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#65054): https://edk2.groups.io/g/devel/message/65054
Mute This Topic: https://groups.io/mt/76625679/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-