[edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Enable Early Serial For DxeCore

[Oliver Smith-Denny]

On 9/7/2023 6:10 AM, Laszlo Ersek wrote:
> (replying on the webui... sorry!)
> 
> The problem is actually embedded in MdePkg and MdeModulePkg.
> 
> - In DxeMain() (and in functions called by DxeMain()), we call DebugLib 
> APIs *before* reaching ProcessLibraryConstructorList().
> 
> - In ArmVirtQemu, we resolve the DXE Core's DebugLib dependency to 
> BaseDebugLibSerialPort (from MdePkg).
> 
> - BaseDebugLibSerialPort has a constructor function 
> (BaseDebugLibSerialPortConstructor()).
> 
> These already suffice for broken DebugLib behavior. APIs of a library 
> class should not be called because the library instance has a chance to 
> initialize.
> 
> The rest is circumstantial. Like, BaseDebugLibSerialPortConstructor 
> calls SerialPortInitialize, but our SerialPortInitialize (in 
> FdtPL011SerialPortLib) does nothing. Well, the latter doesn't need to do 
> anything, because FdtPL011SerialPortLib has its own constructor 
> (FdtPL011SerialPortLibInitialize), thus, if constructors were called 
> properly, then BaseDebugLibSerialPort + FdtPL011SerialPortLib would work 
> properly together, regardless of SerialPortInitialize being empty in the 
> latter.
> 
> Basically the DXE Core has a hidden requirement -- it can only use such 
> DebugLib instances that need no explicit initialization.
> 
> The proposed patch works around the problem by satisfying that hidden 
> requirement one level lower down: in the SerialPortLib instance. The 
> initialization of BaseDebugLibSerialPort is still busted (its 
> constructor is not called, so it cannot call SerialPortInitialize 
> either), but now it is masked, because EarlyFdtPL011SerialPortLib works 
> withouth *both* SerialPortInitialize and construction.
> 
> The real fix would be to make the DXE Core requirement explicit, by 
> introducing separate (dedicated) DebugLib and SerialPortLib *classes* 
> (whose APIs are guaranteed to work without initialization).
> 
> Laszlo

Thanks for the comprehensive breakdown! :). I completely agree that
fixing this at the upper level (and ideally documenting this
requirement) is the better move.

I can drop this patch and take a crack at that. I'm in the last few
weeks leading up to an extended parental leave, so we'll see if I can
squeeze it in prior to then :).

Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108402): https://edk2.groups.io/g/devel/message/108402
Mute This Topic: https://groups.io/mt/101203427/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-