Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: Initialise serial port early in StandaloneMmEntryPoint

["Laszlo Ersek" <lersek@redhat.com>]

On 1/5/24 19:38, Oliver Smith-Denny wrote:
> On 1/5/2024 9:22 AM, levi.yun wrote:
>>
>> Hi Ard :)
>>
>>> So now we will always initialize the serial port in the entrypoint
>>> only because DebugLib might use it later with doing the
>>> initialization.
>>>
>>> That doesn't sound quite correct to me.
>>>
>>> Could you explain why we cannot rely on DebugLib to call the
>>> initializer / constructor at the right time?
>> Because, DebugLib constructor which use serial port is called in
>> StandAloneMmMain function.
>> But, this constrcutor is in _ModuleEntryPoint in StandAloneMmCoreEntry.
>>
>> That means all DEBUG used in _ModuleEntryPoint can use uninitialized
>> serial port.
>> one of typical example is GetSpmVersion function.
>>
>>      _ModuleEntryPoint (in StandAloneMmCoreEntry)
>>
>>       // Hazard Area start
>>          GetSpmVersion
>>              DEBUG (DEBUG_INFO, xxx)  // It could be use uninitalized
>> Serial port.
>>
>>          ...
>>      //  Hazard Area end
>>          ProcessModuleEntryPointList (StandAloneMmMain)
>>              ProcessLibraryConstructorList // Here. call DebugLib
>> constructor with SerialPortIntialize
>>
>> When you see above, I would be clear. between Hazard Area Start to
>> Hazard Area End.
>> DEBUG macro would use uninitailized Serial port if that's not
>> initialized by TF-A.
>>
>> So, It should be call SerialPortInitialized at the _ModuleEntryPoint.
>
> + Laszlo
>
> This sounds very similar to our DxeCore early serial logging discussion
> a couple months ago :).
>
> Laszlo wrote up a good summary here:
> https://edk2.groups.io/g/devel/topic/101203427#109235.
>
> If I am understanding correctly, this would be the "lower left" in
> Laszlo's diagram.
>
> Standalone MM is likely smaller missing window than in DxeCore, but
> some important information could be lost there (like the SPM version
> called out, which could be very important for debugging early crashes).
>
> So this goes back to should be we have a more generic solution for the
> cores to use early logging, by fixing the SerialPortLibs? I'll parse
> this more and reread the old thread further, still paging the info back
> in.

My personal conclusion in that thread was [1], and correspondingly,
commit 5087a0773645 ("ArmVirtPkg/FdtPL011SerialPortLib: initialize
implicitly", 2023-10-07). In the end, the only tractable solution was to
initialize the serial port (hardware, and library instance) exactly
once, in (a) the constructor, or (b) the explicit SerialPortInitialize()
call, or (c) any SerialPortLib API, whichever occurred first. (And (a)
and (b) can be coalesced, because SerialPortInitialize() can be marked
as the constructor for the lib instance.)

[1] http://mid.mail-archive.com/542db9e1-cd28-27a2-3a98-5b0c85cd7c79@redhat.com
    https://edk2.groups.io/g/devel/message/109235

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113386): https://edk2.groups.io/g/devel/message/113386
Mute This Topic: https://groups.io/mt/103540969/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-